Bootstrap GnuPG #24003

alalazo · 2021-05-28T16:49:53Z

depends on #22720
depends on #23889

Modifications:

Bootstrap GnuPG from binaries if not found in the PATH
If can't use binaries, bootstrap GnuPG from sources

becker33 · 2021-08-20T02:13:30Z

lib/spack/spack/test/cmd/gpg.py

+    monkeypatch.setattr(
+        spack.bootstrap, 'ensure_gpg_in_path_or_raise', lambda: None
+    )


Wouldn't it be better to check this by "untrusting" all the bootstrapping methods? That way we're actually checking the proper behavior if gpg isn't available, rather than testing what happens if bootstrapping fails.

See 019ca56

becker33 · 2021-08-20T02:20:20Z

lib/spack/spack/bootstrap.py

+def ensure_gpg_in_path_or_raise():
+    """Ensure gpg or gpg2 are in the PATH or raise."""
+    ensure_executables_in_path_or_raise(
+        executables=['gpg2', 'gpg'], abstract_spec=gnupg_root_spec(),
+    )


We should think about whether this is sustainable to have a method here for each calling site, or whether these methods should be pushed into the callers.

My take on it is the following:

The wrapper function may be pushed on the caller (it's indeed just a convenience to fix parameters in one place)

I think gnupg_root_spec and similar should stay here so that we can collect in the spack.bootstrap module all the specs we may need on bootstrapping

gnupg_root_spec and clingo_root_spec are identical methods, besides the spec string. I think we should have a method get_bootstrap_spec that takes an abstract spec or abstract spec string as an argument. Then the calling site can keep the information that belongs to it ("I need gpg@1.1:" or whatever it is) and call into bootstrap only to turn that abstract information into a bootstrappable spec.

See d2a41f7 I factored together adding a compiler and a target, but I still think it's good to keep in bootstrap.py the wrapping functions, like:

def clingo_root_spec(): """Return the root spec used to bootstrap clingo""" return _root_spec('clingo-bootstrap@spack+python')

The rationale is that the root spec for e.g. clingo should stay consistent across Spack if /when needed in multiple places in the code. So having a function here that returns it seems good to keep the information in a single place.

lib/spack/spack/bootstrap.py

becker33 · 2021-08-20T02:24:40Z

lib/spack/spack/bootstrap.py

@@ -325,6 +447,44 @@ def ensure_module_importable_or_raise(module, abstract_spec=None):
    raise ImportError(msg)


+def ensure_executables_in_path_or_raise(executables, abstract_spec):


Should this be a default argument as it is for modules?

The API is slightly different, since executables accepts a list. As it stands I don't think it needs a default value, but maybe we can revisit this post v0.17.0? If after the release we'll add more dependencies to the bootstrap procedure we may have more data to make the API more consistent.

lib/spack/spack/bootstrap.py

becker33 · 2021-08-20T02:32:32Z

lib/spack/spack/bootstrap.py

+def _executables_in_path(executables):
+    """Return True if at least one of the executables is in PATH,
+    False otherwise.
+    """
+    msg = "[BOOTSTRAP EXECUTABLES {0}] Look if the executables are in the path"
+    executables_str = ', '.join(executables)
+    tty.debug(msg.format(executables_str))
+    found = spack.util.executable.which_string(*executables)
+    if found:
+        msg = "[BOOTSTRAP EXECUTABLES {0}] {1} executable found in PATH"
+        tty.debug(msg.format(executables_str, found))
+        return True
+    return False


Couldn't this method just be replaced with bool(spack.util.executable.which_string(*executables)? I think we're getting a little overly verbose, even for debug mode with some of this.

See dee4605

becker33 · 2021-08-20T02:40:27Z

lib/spack/spack/bootstrap.py

+                current_path = os.environ['PATH']
+
+                os.environ['PATH'] = ':'.join([bin_dir, current_path])
+                if _executables_in_path(executables):
+                    return True
+                os.environ['PATH'] = current_path


Suggested change

current_path = os.environ['PATH']

os.environ['PATH'] = ':'.join([bin_dir, current_path])

if _executables_in_path(executables):

return True

os.environ['PATH'] = current_path

if spack.util.executable.which_string(*executables, path=bin_dir):

spack.util.environment.path_put_first('PATH', [bin_dir])

return True

See dee4605

lib/spack/spack/bootstrap.py

becker33 · 2021-08-20T02:46:10Z

lib/spack/spack/bootstrap.py

+        with spack.config.override(self.mirror_scope):
+            # This index is currently needed to get the compiler used to build some
+            # specs that wwe know by dag hash.
+            spack.binary_distribution.binary_index.regenerate_spec_cache()


We probably need something more robust for this -- do we need to regenerate the spec cache when we're done and switching back to the normal mirro configurations?

I don't think we need to regenerate it, but might be wrong. In any case this method didn't change in this PR. It's what we currently use to bootstrap clingo. Can we postpone making it more robust to a later PR, if there are no major concerns about that?

lib/spack/spack/bootstrap.py

alalazo · 2021-10-26T21:11:21Z

@becker33 Can you review again? I think most points have been addressed (and hopefully the PR will pass unit tests, which are currently still pending).

tgamblin · 2021-10-28T04:51:36Z

@alalazo coverage is still pretty low -- is that avoidable?

alalazo · 2021-10-28T05:51:38Z

coverage is still pretty low -- is that avoidable?

Adding unit tests for this was difficult, so I resorted to e2e tests which are not run with coverage. The one added in this PR is:

spack/.github/workflows/bootstrap.yml

Lines 162 to 188 in 019ca56

    
             ubuntu-gnupg-binaries: 
        
               runs-on: ubuntu-latest 
        
               container: "ubuntu:latest" 
        
               steps: 
        
                 - name: Install dependencies 
        
                   env: 
        
                     DEBIAN_FRONTEND: noninteractive 
        
                   run: | 
        
                     apt-get update -y && apt-get upgrade -y 
        
                     apt-get install -y \ 
        
                         bzip2 curl file g++ gcc patchelf gfortran git gzip \ 
        
                         make patch unzip xz-utils python3 python3-dev tree 
        
                 - uses: actions/checkout@v2 
        
                 - name: Setup repo and non-root user 
        
                   run: | 
        
                     git --version 
        
                     git fetch --unshallow 
        
                     . .github/workflows/setup_git.sh 
        
                     useradd -m spack-test 
        
                     chown -R spack-test . 
        
                 - name: Bootstrap GnuPG 
        
                   shell: runuser -u spack-test -- bash {0} 
        
                   run: | 
        
                     source share/spack/setup-env.sh 
        
                     spack bootstrap untrust spack-install 
        
                     spack -d gpg list 
        
                     tree ~/.spack/bootstrap/store/

I didn't add a macOS tests for binaries since gpg is installed in the base macOS image, but I can try to remove that and see if I can push a working e2e test. Also, I didn't add an e2e test to check building from sources since it takes a while to do so - but let me know if you want to add that.

alalazo · 2021-10-28T10:32:24Z

@tgamblin Added more e2e tests see 9274573 and 20279d7

alalazo added the bootstrap Anything that has to do with Spack building its own dependencies. label May 28, 2021

alalazo force-pushed the features/add_gnupg_to_bootstrap branch from 5de002b to cc52c74 Compare June 1, 2021 18:39

alalazo force-pushed the features/add_gnupg_to_bootstrap branch from cc52c74 to 4ece090 Compare August 6, 2021 12:10

spackbot-app bot added binary-packages commands defaults documentation sbang tests General test capability(ies) utilities workflow labels Aug 6, 2021

alalazo marked this pull request as ready for review August 6, 2021 13:57

alalazo mentioned this pull request Aug 18, 2021

spack style bootstraps over and over #25475

Closed

3 tasks

alalazo force-pushed the features/add_gnupg_to_bootstrap branch from 14d38f2 to 45a8843 Compare August 18, 2021 20:41

alalazo requested review from becker33 and tgamblin August 19, 2021 06:22

alalazo added this to In progress in Spack 0.17.0 Release via automation Aug 19, 2021

becker33 requested changes Aug 20, 2021

View reviewed changes

Spack 0.17.0 Release automation moved this from In progress to Review in progress Aug 20, 2021

alalazo force-pushed the features/add_gnupg_to_bootstrap branch 2 times, most recently from 344a800 to 2780c74 Compare September 16, 2021 14:59

spackbot-app bot added the modules label Sep 16, 2021

alalazo force-pushed the features/add_gnupg_to_bootstrap branch from 2780c74 to 244a307 Compare September 16, 2021 15:03

tgamblin assigned becker33 Oct 25, 2021

alalazo force-pushed the features/add_gnupg_to_bootstrap branch from 244a307 to 9f202b1 Compare October 26, 2021 15:05

alalazo added 2 commits October 28, 2021 10:06

GnuPG: allow bootstrapping from buildcache and sources

6d40965

Add a test to bootstrap GnuPG from binaries

d91e773

alalazo added 6 commits October 28, 2021 10:07

Factor together adding a compiler and a target

0ad30b2

Import spack.user_environment at the module level

e524040

Factor together installing and testing on buildcaches

dabbe65

Don't convert prefix to string

495ad29

Reduce verbosity

b1898fb

Disable bootstrapping in tests

64fcba2

alalazo force-pushed the features/add_gnupg_to_bootstrap branch from 019ca56 to 64fcba2 Compare October 28, 2021 08:08

Add e2e test to bootstrap GnuPG from sources on Ubuntu

9274573

alalazo force-pushed the features/add_gnupg_to_bootstrap branch from e7c0921 to 9274573 Compare October 28, 2021 08:53

Add e2e test to bootstrap GnuPG on macOS

20279d7

alalazo force-pushed the features/add_gnupg_to_bootstrap branch from 73db1b4 to 20279d7 Compare October 28, 2021 09:46

alalazo requested a review from becker33 October 28, 2021 11:55

becker33 approved these changes Nov 3, 2021

View reviewed changes

becker33 merged commit 78c08fc into spack:develop Nov 3, 2021

Spack 0.17.0 Release automation moved this from Review in progress to Done Nov 3, 2021

alalazo deleted the features/add_gnupg_to_bootstrap branch November 3, 2021 06:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bootstrap GnuPG #24003

Bootstrap GnuPG #24003

alalazo commented May 28, 2021 •

edited

becker33 Aug 20, 2021

alalazo Oct 26, 2021

becker33 Aug 20, 2021

alalazo Aug 20, 2021 •

edited

becker33 Aug 20, 2021

alalazo Oct 26, 2021

becker33 Aug 20, 2021

alalazo Oct 26, 2021

becker33 Aug 20, 2021

alalazo Oct 26, 2021

becker33 Aug 20, 2021

alalazo Oct 26, 2021

becker33 Aug 20, 2021

alalazo Oct 26, 2021

alalazo commented Oct 26, 2021

tgamblin commented Oct 28, 2021

alalazo commented Oct 28, 2021 •

edited

alalazo commented Oct 28, 2021

		@@ -325,6 +447,44 @@ def ensure_module_importable_or_raise(module, abstract_spec=None):
		raise ImportError(msg)


		def ensure_executables_in_path_or_raise(executables, abstract_spec):

Bootstrap GnuPG #24003

Bootstrap GnuPG #24003

Conversation

alalazo commented May 28, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alalazo Aug 20, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alalazo commented Oct 26, 2021

tgamblin commented Oct 28, 2021

alalazo commented Oct 28, 2021 • edited

alalazo commented Oct 28, 2021

alalazo commented May 28, 2021 •

edited

alalazo Aug 20, 2021 •

edited

alalazo commented Oct 28, 2021 •

edited