Make installation relocatable, and the installation testable (verilator#4927)

Fixes verilator#4893

Author: gezalore

.github/workflows/build.yml

@@ -17,6 +17,8 @@ env:
   CCACHE_COMPRESS: 1
   CCACHE_DIR: ${{ github.workspace }}/.ccache
   CCACHE_LIMIT_MULTIPLE: 0.95
+  INSTALL_DIR: ${{ github.workspace }}/install
+  RELOC_DIR: ${{ github.workspace }}/relloc
 
 defaults:
   run:
@@ -94,6 +96,7 @@ jobs:
         compiler:
           - { cc: clang, cxx: clang++ }
           - { cc: gcc,   cxx: g++     }
+        reloc: [0]
         suite: [dist-vlt-0, dist-vlt-1, dist-vlt-2, vltmt-0, vltmt-1]
         include:
           # Test with GCC 10 on ubuntu-20.04
@@ -102,14 +105,21 @@ jobs:
           - {os: ubuntu-20.04, compiler: { cc: gcc-10, cxx: g++-10 }, suite: dist-vlt-2}
           - {os: ubuntu-20.04, compiler: { cc: gcc-10, cxx: g++-10 }, suite: vltmt-0}
           - {os: ubuntu-20.04, compiler: { cc: gcc-10, cxx: g++-10 }, suite: vltmt-1}
+          # Test relocated installation - on most common platform only
+          - {os: ubuntu-22.04, compiler: { cc: gcc, cxx: g++ }, reloc: 1, suite: dist-vlt-0}
+          - {os: ubuntu-22.04, compiler: { cc: gcc, cxx: g++ }, reloc: 1, suite: dist-vlt-1}
+          - {os: ubuntu-22.04, compiler: { cc: gcc, cxx: g++ }, reloc: 1, suite: dist-vlt-2}
+          - {os: ubuntu-22.04, compiler: { cc: gcc, cxx: g++ }, reloc: 1, suite: vltmt-0}
+          - {os: ubuntu-22.04, compiler: { cc: gcc, cxx: g++ }, reloc: 1, suite: vltmt-1}
     runs-on: ${{ matrix.os }}
-    name: Test | ${{ matrix.os }} | ${{ matrix.compiler.cc }} | ${{ matrix.suite }}
+    name: Test | ${{ matrix.os }} | ${{ matrix.compiler.cc }} | ${{ matrix.reloc && 'reloc | ' || '' }} ${{ matrix.suite }}
     env:
       CI_BUILD_STAGE_NAME: test
       CI_RUNS_ON: ${{ matrix.os }}
+      CI_RELOC: ${{ matrix.reloc }}
       CC: ${{ matrix.compiler.cc }}
       CXX: ${{ matrix.compiler.cxx }}
-      CACHE_BASE_KEY: test-${{ matrix.os }}-${{ matrix.compiler.cc }}-${{ matrix.suite }}
+      CACHE_BASE_KEY: test-${{ matrix.os }}-${{ matrix.compiler.cc }}-${{ matrix.reloc }}-${{ matrix.suite }}
       CCACHE_MAXSIZE: 100M # Per build per suite (* 5 * 5 = 2500M in total)
       VERILATOR_ARCHIVE: verilator-${{ github.sha }}-${{ matrix.os }}-${{ matrix.compiler.cc }}.tar.gz
     steps:

Makefile.in

@@ -203,11 +203,21 @@ verilator.html:
 verilator.pdf: Makefile
 	$(MAKE) -C docs verilator.pdf
 
-# See uninstall also - don't put wildcards in this variable, it might uninstall other stuff
-VL_INST_BIN_FILES = verilator verilator_bin$(EXEEXT) verilator_bin_dbg$(EXEEXT) verilator_coverage_bin_dbg$(EXEEXT) \
-	verilator_ccache_report verilator_coverage verilator_difftree verilator_gantt verilator_includer verilator_profcfunc
-# Some scripts go into both the search path and pkgdatadir,
-# so they can be found by the user, and under $VERILATOR_ROOT.
+# Public executables intended to be invoked directly by the user
+# Don't put wildcards in these variables, it might cause an uninstall of other stuff
+VL_INST_PUBLIC_SCRIPT_FILES = verilator \
+                              verilator_coverage \
+                              verilator_gantt \
+                              verilator_profcfunc \
+
+VL_INST_PUBLIC_BIN_FILES = verilator_bin$(EXEEXT) \
+                           verilator_bin_dbg$(EXEEXT) \
+                           verilator_coverage_bin_dbg$(EXEEXT) \
+
+# Private executabels intended to be invoked by internals
+# Don't put wildcards in these variables, it might cause an uninstall of other stuff
+VL_INST_PRIVATE_SCRIPT_FILES = verilator_ccache_report \
+                               verilator_includer \
 
 VL_INST_INC_BLDDIR_FILES = \
 	include/verilated_config.h \
@@ -226,19 +236,34 @@ VL_INST_DATA_SRCDIR_FILES = \
 	examples/*/Makefile* \
 	examples/*/vl_* \
 
-installbin:
-	$(MKINSTALLDIRS) $(DESTDIR)$(bindir)
-	( cd ${srcdir}/bin ; $(INSTALL_PROGRAM) verilator $(DESTDIR)$(bindir)/verilator )
-	( cd ${srcdir}/bin ; $(INSTALL_PROGRAM) verilator_coverage $(DESTDIR)$(bindir)/verilator_coverage )
-	( cd ${srcdir}/bin ; $(INSTALL_PROGRAM) verilator_gantt $(DESTDIR)$(bindir)/verilator_gantt )
-	( cd ${srcdir}/bin ; $(INSTALL_PROGRAM) verilator_profcfunc $(DESTDIR)$(bindir)/verilator_profcfunc )
-	( cd bin ; $(INSTALL_PROGRAM) verilator_bin$(EXEEXT) $(DESTDIR)$(bindir)/verilator_bin$(EXEEXT) )
-	( cd bin ; $(INSTALL_PROGRAM) verilator_bin_dbg$(EXEEXT) $(DESTDIR)$(bindir)/verilator_bin_dbg$(EXEEXT) )
-	( cd bin ; $(INSTALL_PROGRAM) verilator_coverage_bin_dbg$(EXEEXT) $(DESTDIR)$(bindir)/verilator_coverage_bin_dbg$(EXEEXT) )
+mkbindirs:
 	$(MKINSTALLDIRS) $(DESTDIR)$(pkgdatadir)/bin
-	( cd ${srcdir}/bin ; $(INSTALL_PROGRAM) verilator_includer $(DESTDIR)$(pkgdatadir)/bin/verilator_includer )
-	( cd ${srcdir}/bin ; $(INSTALL_PROGRAM) verilator_ccache_report $(DESTDIR)$(pkgdatadir)/bin/verilator_ccache_report )
-	( cd ${srcdir}/bin ; $(INSTALL_PROGRAM) verilator_difftree $(DESTDIR)$(pkgdatadir)/bin/verilator_difftree )
+	$(MKINSTALLDIRS) $(DESTDIR)$(bindir)
+
+installbin: | mkbindirs
+	cd $(srcdir)/bin; \
+	for p in $(VL_INST_PUBLIC_SCRIPT_FILES) ; do \
+	  $(INSTALL_PROGRAM) $$p $(DESTDIR)$(pkgdatadir)/bin/$$p; \
+	done
+	cd bin; \
+	for p in $(VL_INST_PUBLIC_BIN_FILES) ; do \
+	  $(INSTALL_PROGRAM) $$p $(DESTDIR)$(pkgdatadir)/bin/$$p; \
+	done
+	cd $(srcdir)/bin; \
+	for p in $(VL_INST_PRIVATE_SCRIPT_FILES) ; do \
+	  $(INSTALL_PROGRAM) $$p $(DESTDIR)$(pkgdatadir)/bin/$$p; \
+	done
+
+installredirect: installbin | mkbindirs
+	cp ${srcdir}/bin/redirect ${srcdir}/bin/redirect.tmp
+	perl -p -i -e 'use File::Spec;' \
+	           -e' $$path = File::Spec->abs2rel("$(realpath $(DESTDIR)$(pkgdatadir)/bin)", "$(realpath $(DESTDIR)$(bindir))");' \
+	           -e 's/RELPATH.*/"$$path";/g' -- "${srcdir}/bin/redirect.tmp"
+	cd $(srcdir)/bin; \
+	for p in $(VL_INST_PUBLIC_SCRIPT_FILES) $(VL_INST_PUBLIC_BIN_FILES) ; do \
+	  $(INSTALL_PROGRAM) redirect.tmp $(DESTDIR)$(bindir)/$$p; \
+	done
+	rm ${srcdir}/bin/redirect.tmp
 
 # Man files can either be part of the original kit, or built in current directory
 # So important we use $^ so VPATH is searched
@@ -281,8 +306,11 @@ installdata:
 
 # We don't trust rm -rf, so rmdir instead as it will fail if user put in other files
 uninstall:
-	-cd $(DESTDIR)$(bindir) && rm -f $(VL_INST_BIN_FILES)
-	-cd $(DESTDIR)$(pkgdatadir)/bin && rm -f $(VL_INST_BIN_FILES)
+	-cd $(DESTDIR)$(bindir) && rm -f $(VL_INST_PUBLIC_SCRIPT_FILES)
+	-cd $(DESTDIR)$(bindir) && rm -f $(VL_INST_PUBLIC_BIN_FILES)
+	-cd $(DESTDIR)$(pkgdatadir)/bin && rm -f $(VL_INST_PUBLIC_SCRIPT_FILES)
+	-cd $(DESTDIR)$(pkgdatadir)/bin && rm -f $(VL_INST_PUBLIC_BIN_FILES)
+	-cd $(DESTDIR)$(pkgdatadir)/bin && rm -f $(VL_INST_PRIVATE_SCRIPT_FILES)
 	-cd $(DESTDIR)$(mandir)/man1 && rm -f $(VL_INST_MAN_FILES)
 	-cd $(DESTDIR)$(pkgdatadir) && rm -f $(VL_INST_INC_BLDDIR_FILES)
 	-cd $(DESTDIR)$(pkgdatadir) && rm -f $(VL_INST_INC_SRCDIR_FILES)
@@ -311,7 +339,7 @@ uninstall:
 	-rmdir $(DESTDIR)$(pkgconfigdir)
 
 install: all_nomsg install-all
-install-all: installbin installman installdata install-msg
+install-all: installbin installredirect installman installdata install-msg
 
 install-here: installman info
 

bin/redirect

@@ -0,0 +1,22 @@
+#!/usr/bin/env perl
+######################################################################
+#
+# Copyright 2003-2024 by Wilson Snyder. This program is free software; you
+# can redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+#
+######################################################################
+
+require 5.006_001;
+use warnings;
+use FindBin qw($RealBin $RealScript);
+use strict;
+
+my $relpath = RELPATH;  # Substituted during Verilator 'make install'
+
+unshift @ARGV, $RealScript;
+
+exec { "$RealBin/$relpath/$RealScript" } @ARGV;
+die "%Error: Exec failed,";

bin/verilator

@@ -15,6 +15,7 @@ use Getopt::Long;
 use FindBin qw($RealBin $RealScript);
 use IO::File;
 use Pod::Usage;
+use Cwd qw(realpath);
 
 use strict;
 use vars qw($Debug @Opt_Verilator_Sw);
@@ -67,6 +68,18 @@ if (! GetOptions(
     pod2usage(-exitstatus => 2, -verbose => 0);
 }
 
+my $verilator_root = realpath("$RealBin/..");
+if (defined $ENV{VERILATOR_ROOT}) {
+    if ((!-d $ENV{VERILATOR_ROOT}) || $verilator_root ne realpath($ENV{VERILATOR_ROOT})) {
+        warn "%Error: verilator: VERILATOR_ROOT is set to inconsistent path. Suggest leaving it unset.\n";
+        warn "%Error: VERILATOR_ROOT=$ENV{VERILATOR_ROOT}\n";
+        exit 1;
+    }
+} else {
+    print "export VERILATOR_ROOT='$verilator_root'\n" if $Debug;
+    $ENV{VERILATOR_ROOT} = $verilator_root;
+}
+
 if ($opt_gdbbt && !gdb_works()) {
     warn "-Info: --gdbbt ignored: gdb doesn't seem to be working\n" if $Debug;
     $opt_gdbbt = 0;
@@ -151,30 +164,13 @@ sub debug {
 # Builds
 
 sub verilator_bin {
-    my $bin = "";
-    # Use VERILATOR_ROOT if defined, else assume verilator_bin is in the search path
     my $basename = ($ENV{VERILATOR_BIN}
                     || ($Debug ? "verilator_bin_dbg" : "verilator_bin"));
-    if (defined($ENV{VERILATOR_ROOT})) {
-        my $dir = $ENV{VERILATOR_ROOT};
-        if (-x "$dir/bin/$basename"
-            || -x "$dir/bin/$basename.exe") {  # From a "make install" into VERILATOR_ROOT
-            $bin = "$dir/bin/$basename";
-        } else {
-            $bin = "$dir/$basename";  # From pointing to kit directory
-        }
+    if (-x "$RealBin/$basename" || -x "$RealBin/$basename.exe") {
+        return "$RealBin/$basename";
     } else {
-        if (-x "$RealBin/$basename"
-            || -x "$RealBin/$basename.exe") {
-            $bin = "$RealBin/$basename";  # From path/to/verilator with verilator_bin installed
-        } else {
-            $bin = $basename;  # Find in PATH
-        }
-        # Note we don't look under bin/$basename which would be right if running
-        # in the kit dir. Running that would likely break, since
-        # VERILATOR_ROOT wouldn't be set and Verilator won't find internal files.
+        return $basename;  # Find in PATH
     }
-    return $bin;
 }
 
 #######################################################################

ci/ci-script.bash

@@ -40,7 +40,7 @@ if [ "$CI_BUILD_STAGE_NAME" = "build" ]; then
 
   if [ "$COVERAGE" != 1 ]; then
     autoconf
-    ./configure --enable-longtests --enable-ccwarn
+    ./configure --enable-longtests --enable-ccwarn --prefix="$INSTALL_DIR"
     ccache -z
     "$MAKE" -j "$NPROC" -k
     # 22.04: ccache -s -v
@@ -87,23 +87,37 @@ elif [ "$CI_BUILD_STAGE_NAME" = "test" ]; then
   # Run sanitize on Ubuntu 22.04 only
   [ "$CI_RUNS_ON" = 'ubuntu-22.04' ] && sanitize='--sanitize' || sanitize=''
 
+  TEST_REGRESS=test_regress
+  if [ "$CI_RELOC" == 1 ]; then
+     # Testing that the installation is relocatable.
+     "$MAKE" install
+     mkdir -p "$RELOC_DIR"
+     mv "$INSTALL_DIR" "$RELOC_DIR/relocated-install"
+     export VERILATOR_ROOT="$RELOC_DIR/relocated-install/share/verilator"
+     TEST_REGRESS="$RELOC_DIR/test_regress"
+     mv test_regress "$TEST_REGRESS"
+     # Feeling brave?
+     find . -delete
+     ls -la .
+  fi
+
   # Run the specified test
   ccache -z
   case $TESTS in
     dist-vlt-0)
-      "$MAKE" -C test_regress SCENARIOS="--dist --vlt $sanitize" DRIVER_HASHSET=--hashset=0/3
+      "$MAKE" -C "$TEST_REGRESS" SCENARIOS="--dist --vlt $sanitize" DRIVER_HASHSET=--hashset=0/3
       ;;
     dist-vlt-1)
-      "$MAKE" -C test_regress SCENARIOS="--dist --vlt $sanitize" DRIVER_HASHSET=--hashset=1/3
+      "$MAKE" -C "$TEST_REGRESS" SCENARIOS="--dist --vlt $sanitize" DRIVER_HASHSET=--hashset=1/3
       ;;
     dist-vlt-2)
-      "$MAKE" -C test_regress SCENARIOS="--dist --vlt $sanitize" DRIVER_HASHSET=--hashset=2/3
+      "$MAKE" -C "$TEST_REGRESS" SCENARIOS="--dist --vlt $sanitize" DRIVER_HASHSET=--hashset=2/3
       ;;
     vltmt-0)
-      "$MAKE" -C test_regress SCENARIOS=--vltmt DRIVER_HASHSET=--hashset=0/2
+      "$MAKE" -C "$TEST_REGRESS" SCENARIOS=--vltmt DRIVER_HASHSET=--hashset=0/2
       ;;
     vltmt-1)
-      "$MAKE" -C test_regress SCENARIOS=--vltmt DRIVER_HASHSET=--hashset=1/2
+      "$MAKE" -C "$TEST_REGRESS" SCENARIOS=--vltmt DRIVER_HASHSET=--hashset=1/2
       ;;
     coverage-all)
       nodist/code_coverage --stages 1-

test_regress/Makefile

@@ -16,7 +16,7 @@
 default: test
 
 # This must point to the root of the VERILATOR kit
-VERILATOR_ROOT := $(shell pwd)/..
+VERILATOR_ROOT ?= $(shell pwd)/..
 export VERILATOR_ROOT
 
 # Pick up PERL and other variable settings

test_regress/driver.pl

@@ -12,6 +12,8 @@ BEGIN
     }
     $ENV{MAKE} ||= "make";
     $ENV{CXX} ||= "c++";
+    !defined $ENV{TEST_REGRESS} or die "TEST_REGRESS environment variable is already set";
+    $ENV{TEST_REGRESS} = Cwd::getcwd();
 }
 
 use Getopt::Long;
@@ -2484,7 +2486,7 @@ sub _vcd_read {
 our $_Cxx_Version;
 
 sub cxx_version {
-    $_Cxx_Version ||= `$ENV{MAKE} -C $ENV{VERILATOR_ROOT}/test_regress -f Makefile print-cxx-version`;
+    $_Cxx_Version ||= `$ENV{MAKE} -C $ENV{TEST_REGRESS} -f Makefile print-cxx-version`;
     return $_Cxx_Version;
 }
 

test_regress/t/t_a1_first_cc.pl

@@ -16,7 +16,7 @@
 
 $DEBUG_QUIET = "--debug --debugi 0 --gdbbt --no-dump-tree";
 
-run(cmd => ["perl", "../bin/verilator", $DEBUG_QUIET, "-V"],
+run(cmd => ["perl", "$ENV{VERILATOR_ROOT}/bin/verilator", $DEBUG_QUIET, "-V"],
     verilator_run => 1,
     );
 

test_regress/t/t_a5_attributes_include.pl

@@ -10,8 +10,13 @@
 
 scenarios(dist => 1);
 rerunnable(0);
+
+my $root = "..";
+
 if ($ENV{VERILATOR_TEST_NO_ATTRIBUTES}) {
     skip("Skipping due to VERILATOR_TEST_NO_ATTRIBUTES");
+} elsif (!-r "$root/.git") {
+    skip("Not in a git repository");
 } else {
     check();
 }