@scheibelp OK I've done an initial once-over pass merging from #7846. Now time to try it out, test it, and work out the kinks. I don't think this will be so bad... most of the code from #7846 is now-obsolete Alpha test Environments stuff. The code relevant to implementing Spack Setup (as opposed to the UI) was pretty self-contained.

@scheibelp Spack Setup looks like this works now, can you take a look at it? I've tried stuff like:

spack install --setup mypackage mypackage

The tricky logic just carried over from the previous (well-tested) PR, so I have relatively high confidence in it. This PR is now ready for:

1. Unit tests. Tests should show at least two packages being setup in a single spack install command. They should also show how setup can be specified in the environment's YAML file (something I have not yet tested; but I believe was tested before I got to this PR).
2. Docs
3. Real-world user tests. IF we think this PR is going to be merged, I'll do that once the government shutdown ends. (Otherwise, I'd rather wait till a final-version PR before re-doing my Spack build, again, to conform to the new way of doing things.) If I can build my stuff with this, I'll have high confidence in saying it's ready to merge.

Backing up, recent conversations with @tgamblin suggest a possible future in which Spack is a LOT better at defining and using environments. Advantages would be:

1. Ability to use Spack compiler wrappers outside of Spack, which would allow the spconfig scripts generated by this PR to replicated Spack exactly (they currently don't use wrappers).
2. More general way of exporting Spack build environments, which could be good for more than just CMake.

Wary of making the perfect the enemy of the good, I think we should merge what we have now (that's been waiting for a long time and that people want); and then go on to something better.

@davydden

I have updated the prefix checking logic to respect #10403 (comment)

I also updated write_spconfig to be invoked inside of build_process. This is to avoid editing the current environment when invoking write_spconfig

A couple questions:

• It occurs to me there may be cases where a dependency is marked "setup" but, that editing the associated package.py (for the dependency) won't re-write the spconfig file. E.g. if you have X->Y->Z, Z is setup, and Y doesn't change at all (but is installed).
• This doesn't actually perform the install, so if you have X->Y, and you set up Y, it seems that X may fail if the needed Y artifacts are not present.

Real-world user tests.

I gave it a go with spack install --setup dealii dealii and at least it concretized and ran through without problems.

The configuration via (in this case) $python dealii-setup.py <path-to-source> worked flawlessly, I am currently building the package from my local sources, but I don't expect any negative surprises...

I would say even a very minimalistic doc (i.e. an extension of --setup help message saying that the command creates a <package-name>-setup.py, which can be used by providing source location) would be enough to let people try.

The only thing from my perspective is the following: If a provided spec (in my case root of DAG) was not installed yet, spack install --setup will leave an empty directory in opt and register this spec as being present/installed. I usually start by installing a package via Spack and then re-configuring/building for development, but if a given spec was not installed yet, that could be confusing for users. I would say an option to only create xyz-setup.py and let Spack think that the package is not installed is good to have. But this shall not hold this PR in any way.

Thank you very much for pushing this feature @citibeth @scheibelp 👍

move write_spconfig into build_process (to avoid affecting current environment)

@scheibelp Can you explain more what you mean by this? Not sure what "current environment" refers to. Does this change remove the feature that writes the x-setup.py files into the environment's directory?

move write_spconfig into build_process (to avoid affecting current environment)

Can you explain more what you mean by this? Not sure what "current environment" refers to. Does this change remove the feature that writes the x-setup.py files into the environment's directory?

By "current environment" I mean the shell where the user is running Spack: if you execute setup_package in the "main" spack process (i.e. not inside of build_process or within some subprocess you define) then it will modify the shell variables of your current process. That isn't necessarily an error but could lead to strange errors.

Is that clearer?

Does 0ebabc1 mean that now, it is no longer an error for Spack to try to create an install directory that already exists? Why the change?

Generally Spack claims control over all Spack prefixes. Part of its logic automatically deletes the install prefix if it exists but the package isn't registered in the DB. The AlreadyExists exception was vestigial. This PR adds one case where that shouldn't occur, but that is handled by the conditional execution of check_for_unfinished_installation.

I don't understand the rationale for 65c008e. Writing the xxx-setup.py ("spconfig") file doesn't change any environments. It just writes a text file.

Writing the file did not change the environment, but executing setup_package did.

    def write_spconfig(self, spack_env, spconfig_fname, dirty):
        # Execute all environment setup routines.
        spack.build_environment.setup_package(self, dirty)      <------ potentially problematic line that was removed by 65c008e

OK... the bottom line is, if you're doing --setup on a package, then Spack should not delete the install directory. Spack Setup cedes control of the install directory to the user. Is that the case in the current incarnation?

That is the intent, and I think this iteration handles it. It should be tested (along with any other "really important" property).

It occurs to me there may be cases where a dependency is marked "setup" but, that editing the associated package.py (for the dependency) won't re-write the spconfig file. E.g. if you have X->Y->Z, Z is setup, and Y doesn't change at all (but is installed).

This doesn't actually perform the install, so if you have X->Y, and you set up Y, it seems that X may fail if the needed Y artifacts are not present.

In my use of Spack Setup so far, I've observed an (unwritten) rule that if X->Y and Y is setup, then so is X. That has worked for me so far because the packages I'm developing are at the top of the DAG. That's probably the most common case. Currently, there is no check for this property. Adding a check (and error message) at the point of concretization would be a positive addition to this PR, and not very hard.

One could imagine situations that don't follow this property. For example, you're trying to debug a basic library (say, NetCDF) deep in the DAG, and so you do setup on that library; but everything else you want to do regular spack install on. As long as you're using shared libraries and not changing header files, you can quickly re-do runs of your top-level program just by rebuilding your low-level library (NetCDF in this case); without having to rebuild everything on top of it every time it changes.

To support this kind of use case, Spack would need a "phased install" approach. You would run spack install once. Spack would go through the install, working its way up tree, assuming that all setup packages are not built (and therefore it can't do anything that depends on a setup package). Once it can't make any further progress, it stops and tells the user to install the given setup pacakge(s) that stopped the run. Once the user has done this, installation would continue. Rinse, repeat until everything has been installed.

This feels like doing a git rebase when the process has to stop occasionally for manual difference resolution.

I believe we should seriously consider this kind of feature, but not in this PR. For this PR, a simple check that "A->B and (B is setup) implies A is setup" would be sufficient.

Would it be helpful, either now or in a future PR, to associate each setup package with its location? For now, I'm not sure how setup dependents find setup dependencies (since they aren't built in the Spack prefix).

Not sure what you mean by the "location" of a setup package? Do you mean the install location or the location of the source? I don't think Spack needs to keep track of the location of the source; after all, for non-setup packages, it deletes the source code!

@davydden

The only thing from my perspective is the following: If a provided spec (in my case root of DAG) was not installed yet, spack install --setup will leave an empty directory in opt and register this spec as being present/installed. I usually start by installing a package via Spack and then re-configuring/building for development, but if a given spec was not installed yet, that could be confusing for users. I would say an option to only create xyz-setup.py and let Spack think that the package is not installed is good to have. But this shall not hold this PR in any way.

You have NO IDEA what a headache that issue has been. If you don't create the directory, various things don't work. No, I don't remember. If someone wants to address the issue, they can start by removing the directory creation, then see what happens.

@scheibelp @tgamblin
Everybody agrees this PR is a good idea, and yet it has languished for over a year (going on 18 months)! Now it needs updating again. How can we get it merged NOW? Who has the authority to merge it?

It's great that #11372 got merged. Can we get this one merged too? Its kind of gumming up a lot of things.

@scheibelp @tgamblin @alalazo

What can I do to get this merged? It has, yet again, fallen out of sync with develop and now will need more work. The original PR is now 2 years old, and critical to my work. Lack of merging here is preventing me from updating to the latest Spack.

Can we get this merged? How can I help?

@citibeth: sorry, it’s been busy with releases and such. One issue with this PR is that is has no tests. The patch coverage is 50%. If you can get it to 90+%, so that spack setup will be in our regression tests, we’ll get it merged for 0.14. Sound good?

I took a look at lending a hand on this, but it doesn't rebase cleaning onto the current develop. Looks like some of the yaml config bits have changed (perhaps including ideas/flags from the PR?) and I don't have the context to figure out how it's all intended to fit together.

@citibeth -- if you can get it rebased onto develop, I can find the TUIT's to either set up some tests or at least frame them out so that you can fill in the thought-requiring bits. I'm going to be out of pocket between now and Christmas, but should have time after.