This looks broadly similar to my code mentioned in #3245. One thing I did add later was removing absolute references from merged easy-install.pth as dependencies were activated anyway and the file ends up with lots of duplicates. Maybe this does that as well, but I did not see it.

This is clearly the right direction. spack activate fundamentally breaks Spack, for the reasons given above; and this is a way to do a similar thing that does not break Spack. (I personally have been happy with loading modules).

@citibeth wrote:

[...] (I personally have been happy with loading modules).

We started to get quite unhappy when we crossed the 50 entries per environment variable barrier (we basically dropped all system-installed software and rely 100% on spack for all our software dependencies).
→ You cannot open manpages anymore, loading python gets incredible slow, …

Due to @obreitwi's change we (@electronicvisions) are all happy again ;)

@citibeth yeah, we use NFS and it is unbearable...

Just added a missing feature that locally and globally activated extensions are handled correctly (forgot about this as we do not plan on using any globally activated extensions).

@paulhopkins how about using os.path.realpath to normalize all paths, make sure there are no duplicates and finally os.path.relpath to generate a pretty easy_install.pth file?

Some further functionality (plus a rebase on current develop branch):

• Added -i/--ignore-conflicts command line option to print warnings
  instead of raising an exception whenever conflicting files are found.
  Please note that this only works for linking non-extensions as the
  activation-procedure should take care of conflicting files in
  extensions.

• To that end, LinkTree now has the option to report_conflicts instead
  of raising assertions when merge-ing.

• When activating a python extension, the easy_install.pth file is
  pruned from duplicated lines (while preserving the original order).

Actually, I just noticed #3587, which solves the problem of filesystem views being unusable with python extensions in general, still, we need this PR to have different sets of activated extensions in each view within one spack installation.

I've spoken with @tgamblin and I will be reviewing this over the next couple days.

Two initial notes:

• I notice you generally check for things like if path_view is None vs. if not path_view - are there times when the empty string would be a valid path_view?
• When I build "git" in your PR and then try spack view soft sandbox/ git I get the error Specs passed to a DirectoryLayout must be concrete!. I don't think this is on account of the merge conflict (that appears to just be imports). The original view logic appears to be able to do this. If you're not sure I can look into it myself.

Thank you for taking the time.

• You are correct, there is no point in differentiating between None and "", an empty path is never a valid location. I have refactored accordingly.

• We did, however, encounter something similar a while back, but I did not find the time to properly investigate:
  In our case, the error occurred because the .extendee_spec of a given package was still present, even though the package was not configured to be an extension. If I understood the code correctly, this is not supposed to happen.
  Upon (quick) second investigation it might also be the case that the flatten(...)-method should simply use concretized() instead…

I will test this out as soon as I have time!

When I build "git" in your PR and then try spack view soft sandbox/ git I get the error Specs passed to a DirectoryLayout must be concrete!.

If you modify check_is_extension like so then this issue should be resolved:

    if not ext_spec.package.is_extension:
        return False

    # TODO: Update this once more than one extendee is allowed
    #  for spec in ext_spec.package.extendees:
    spec = ext_spec.package.extendee_spec
    if spec not in ext_spec:
        print "not actually an extension", ext_spec.name
        return False

Main issue is that package.is_extension checks whether the package could be an extension. In the case where I was trying to link in git, the libxml2 package conditionally extends python but in my case actually doesn't, so when you grab extendee_spec you're not actually getting an installed package.

EDIT: my mistake - Package should deal with this (and you shouldn't have to)

In our case, the error occurred because the .extendee_spec of a given package was still present, even though the package was not configured to be an extension. If I understood the code correctly, this is not supposed to happen.

Oh I see what you're saying there. Yeah in this case that is more likely something that should be handled in package.py

When I build "git" in your PR and then try spack view soft sandbox/ git I get the error Specs passed to a DirectoryLayout must be concrete!.

I edited my follow-up comment but I also wanted to make explicit that this is not an issue with this PR.

Upon (quick) second investigation it might also be the case that the flatten(...)-method should simply use concretized() instead…

It shouldn't need to call normalized or concretized (i.e. just use spec.traverse()) since at that point the function is working with specs resolved with the disambiguate_specs function (which fetches installed specs from the database).

I can avoid the error I mentioned by updating Package. I'll try to get a PR out for it by the end of tomorrow.

I just noticed #3587, which solves the problem of filesystem views being unusable with python extensions in general,

I don't think #3587 fully resolves this, since you take the effort to use activate logic for extensions you benefit from whatever files they choose not to merge. In the case of python modules that includes "site.py"

I'm noticing an extra file that is added is .spack-empty - do you know where that comes from?

When I merge #3853 into your PR I can add git to a view

apparently there are still some cases left where easy_install.pths need to be merged.

In that case, does my note about realpath make sense?

Yes, we should just remove duplicate lines from the resulting easy_install.pth, imho.

To be clear I think there is an issue beyond other than duplicate lines. For example if I install py-babel and then run

./bin/spack view soft sandbox/ py-babel

With your PR, then sandbox/lib/python2.7/site-packages/easy-install.pth looks like:

/g/g17/scheibel/repos/spack/Babel-2.3.4-py2.7.egg
/g/g17/scheibel/repos/spack/opt/spack/linux-rhel6-x86_64/gcc-4.4.7/py-pytz-2016.10-f54ik3bezdsnnyfvixrtvreb5t5vytbc/lib/python2.7/site-packages/pytz-2016.10-py2.7.egg
/g/g17/scheibel/repos/spack/pytz-2016.10-py2.7.egg
import sys; new=sys.path[sys.__plen:]; del sys.path[sys.__plen:]; p=getattr(sys,'__egginsert',0); sys.path[p:p]=new; sys.__egginsert = p+len(new)

The problematic line IMO is /g/g17/scheibel/repos/spack/Babel-2.3.4-py2.7.egg - that path doesn't exist. I'm pretty sure it looks like that because your PR calls realpath on a path which starts with ./ while Spack is being run from /g/g17/scheibel/repos/spack/

Yes, you are completely correct.

I removed the call to realpath and just make sure any duplicate lines are removed (plus a rebase on current upstream/develop).

Okay, thanks, in the meantime I'll see to it that Travis has nothing to complain about.

Generally I like the reorganization but I'm still seeing the issue I mentioned earlier:

One thing I did notice when trying spack view soft sandbox/ py-pytz was that the pytz module files were showing up in sandbox/.spack/python/lib/python2.7/site-packages/pytz (where I would instead expect them to show up in sandbox/lib/python2.7/site-packages/pytz)

Oh, I guess I couldn't see the forest for the trees. All extensions were linked into the meta-folder of the extendee instead of the root of the view. Should be fixed now.

I'll also start thinking of tests to suggest and I'll add an update on that by the end of tomorrow.

• Firstly I suggested testing write_easy_install_pth at Allow for extensions on a per filesystem view basis #3227 (comment). This might be easier to test if that function were refactored to take the names of the .pth files to merge, and returned the contents of the merged file (or if that were to be placed in a helper method).
• Some existing mock packages already create installation files. You could test installing and then adding any of them to a view.
• There are a couple of mock extension packages, but neither of them define multiple versions. IMO you could modify either one to have 2 versions, install both, and then confirm that both cannot be added to a view.
• Install a dependent, add it (plus the dependency) to a view, remove the dependency from the view. The functionality to decide on what gets removed could be refactored into its own function to make this trivial (so you don't necessarily have to install things, just make sure the specs are calculated properly).

I'll keep thinking on it. Let me know if you think these aren't useful or aren't in scope for your PR. Also if you think the tests are useful but want to chat about how to implement them let me know.

Okay, just updated the branch. Now global state is not modified, the status and remove command are a bit more lenient when it comes to specifying specs (basically names are enough) and the color-output is disabled when there is no TTY attached to stdout.

After making my head hurt at first (I wanted to not remove dependencies that have non-user-specified dependents) I decided to make it nice and simple:

• If dependencies are to be removed they are added to the list of specs to remove.
• If dependents are to be removed all dependents are identified and removed as well (this includes dependents of dependencies of user-specified packages). A warning is printed.
• If dependents are not to be removed, we identify them anyway and print a warning to the user (even without verbose output) that the packages will become unusable.

Basically I don't see users removing packages from views that much anyway. It is just too easy to delete everything and add exactly what you want. Now it is the most conservative approach (which happens to delete more rather than having unusable packages linger).

If the PR is fine otherwise I'll have a closer look at the tests in the coming days.

Basically I don't see users removing packages from views that much anyway. It is just too easy to delete everything and add exactly what you want.

Makes sense to me

If the PR is fine otherwise I'll have a closer look at the tests in the coming days.

Added a few more comments. IMO they are all minor issues.

After having had a quick look at the testing routines, some questions/thoughts came up:

• I think write_easy_install_pth should be tested and refactored out; especially given the recent rewrites (fix easy-install.pth #3569, Restore newlines to easy-install.pth files. #3583). Although, partly thanks to Don't build eggs. #3587, it is not that much of an issue and really outside the scope of this PR (which is being able to activate different sets of extensions from the same spack-repository into different filesystem views). It was its primary motivation, though.

• The builtin.mock-python package is missing its do_activate function. How should proper functionality of the actual package be verified in the current testing framework?

• On what level should be tested? Only user based input with builtin.mock as repository or directly on the level of FilesystemView? In the latter case, I am torn between adding some packages to builtin.mock and having mocked Specs and Package generated on the fly (as with the install command tests) that just install an adjustable set of random-dummy files based on their version-hash.

• Even more general: Do we have to test for proper linking of files given the fact that LinkTree is already covered by tests? Would testing conflict-detection, dependency-resolutoin and proper removal of dependencies/dependents be enough when performed on "specs without install files"-level?

• If linking should be tested, are their testing routines that allow builtin.mock to contain installed specs (so that these could actually be linked to a temporary filesystem view)?

The builtin.mock-python package is missing its do_activate function. How should proper functionality of the actual package be verified in the current testing framework?

IMO you don't need to test Python.activate. Even testing write_easy_install_pth is a bit beyond the norm. That being said I think you can access the "real" python easily if you don't use the install_mockery fixture.

Do we have to test for proper linking of files given the fact that LinkTree is already covered by tests?

IMO no

Would testing conflict-detection, dependency-resolution and proper removal of dependencies/dependents be enough when performed on "specs without install files"-level?

I could see how the last two could be tested on specs without install files. The first one would be harder assuming we're talking about file conflict detection (vs. detecting whether some other version of a package has been added to a view), although IMO you don't have to work with specs (could just be one of the link_tree tests)

On what level should be tested? Only user based input with builtin.mock as repository or directly on the level of FilesystemView?

If by "Only user based input with builtin.mock" you mean invocations of cmd.view I don't think it has to be at that level. Direct testing of filesystem_view seems good to me.

In the latter case, I am torn between adding some packages to builtin.mock and having mocked Specs and Package generated on the fly (as with the install command tests) that just install an adjustable set of random-dummy files based on their version-hash.

While test/cmd/install uses mock Specs and packages test/install does not. IMO builtin.mock would be easier here given that the view logic calls more functions but either way would be fine.

I've rebased this and split it into multiple commits in #5415.

#5415 went in. Can we close this?

Superseded by #5415