Concretizer reuse doesn't understand compiler flags compatibility

carns commented 2 years ago

Steps to reproduce

I'm using the json-c package with address sanitizer options as a simple example, but this bug isn't particular to that package or those options, it's just an illustration.

I start by creating two different environment configurations, they are identical except that one sets custom cflags and ldflags for development/debugging purposes:

carns-x1-7g ~/w/d/spack-reuse-cflags-2022-02> ls
json-c-asan.yaml  json-c.yaml
carns-x1-7g ~/w/d/spack-reuse-cflags-2022-02> cat json-c.yaml
spack:
  specs:
    - json-c
  concretization: together
  view: true
carns-x1-7g ~/w/d/spack-reuse-cflags-2022-02> cat json-c-asan.yaml
spack:
  specs:
    - json-c cflags="-fsanitize=address -fno-omit-frame-pointer -g -Wall" ldflags="-fsanitize=address"
  concretization: together
  view: true

I then create an environment using the yaml that specifies custom cflags and ldflags:

carns-x1-7g ~/w/d/spack-reuse-cflags-2022-02> 
spack env create json-c-asan json-c-asan.yaml
==> Updating view at /home/carns/working/src/spack/var/spack/environments/json-c-asan/.spack-env/view
==> Created environment 'json-c-asan' in /home/carns/working/src/spack/var/spack/environments/json-c-asan
==> You can activate this environment with:
==>   spack env activate json-c-asan
carns-x1-7g ~/w/d/spack-reuse-cflags-2022-02> spack env activate json-c-asan
carns-x1-7g ~/w/d/spack-reuse-cflags-2022-02> spack install
==> Concretized json-c cflags="-fsanitize=address -fno-omit-frame-pointer -g -Wall" ldflags="-fsanitize=address"
 -   4i2cra2  json-c@0.15%gcc@11.2.0 cflags="-fsanitize=address -fno-omit-frame-pointer -g -Wall" ldflags="-fsanitize=address" ~ipo build_type=RelWithDebInfo arch=linux-ubuntu21.10-skylake
 -   u7tmc35      ^cmake@3.18.4%gcc@11.2.0 cflags="-fsanitize=address -fno-omit-frame-pointer -g -Wall" ldflags="-fsanitize=address" ~doc+ncurses+openssl+ownlibs~qt build_type=Release patches=bf695e3febb222da2ed94b3beea600650e4318975da90e4a71d6f31a6d5d8c3d arch=linux-ubuntu21.10-skylake

==> Installing environment json-c-asan
[+] /usr (external cmake-3.18.4-u7tmc355fevlgqmbkix5ryfiurkatb6v)
==> Installing json-c-0.15-4i2cra2gmog2tf6qstkw35ox4jbqbtfy
==> No binary for json-c-0.15-4i2cra2gmog2tf6qstkw35ox4jbqbtfy found: installing from source
==> Using cached archive: /home/carns/working/src/spack/var/spack/cache/_source-cache/archive/b8/b8d80a1ddb718b3ba7492916237bbf86609e9709fb007e7f7d4322f02341a4c6.tar.gz
==> No patches needed for json-c
==> json-c: Executing phase: 'cmake'
==> json-c: Executing phase: 'build'
==> json-c: Executing phase: 'install'
==> json-c: Successfully installed json-c-0.15-4i2cra2gmog2tf6qstkw35ox4jbqbtfy
  Fetch: 0.00s.  Build: 11.42s.  Total: 11.42s.
[+] /home/carns/working/src/spack/opt/spack/linux-ubuntu21.10-skylake/gcc-11.2.0/json-c-0.15-4i2cra2gmog2tf6qstkw35ox4jbqbtfy
==> Updating view at /home/carns/working/src/spack/var/spack/environments/json-c-asan/.spack-env/view
carns-x1-7g ~/w/d/spack-reuse-cflags-2022-02> spack env deactivate

And finally I create another environment, without the custom cflags or ldflags, and install using the --reuse option:

carns-x1-7g ~/w/d/spack-reuse-cflags-2022-02> 
spack env create json-c json-c.yaml
==> Updating view at /home/carns/working/src/spack/var/spack/environments/json-c/.spack-env/view
==> Created environment 'json-c' in /home/carns/working/src/spack/var/spack/environments/json-c
==> You can activate this environment with:
==>   spack env activate json-c
carns-x1-7g ~/w/d/spack-reuse-cflags-2022-02> spack env activate json-c
carns-x1-7g ~/w/d/spack-reuse-cflags-2022-02> spack install --reuse
==> Concretized json-c
[+]  4i2cra2  json-c@0.15%gcc@11.2.0 cflags="-fsanitize=address -fno-omit-frame-pointer -g -Wall" ldflags="-fsanitize=address" ~ipo build_type=RelWithDebInfo arch=linux-ubuntu21.10-skylake

==> Installing environment json-c
==> All of the packages are already installed
==> Updating view at /home/carns/working/src/spack/var/spack/environments/json-c/.spack-env/view

The second environment reused the package from the previous environment that had been compiled with address sanitizer options, even though they weren't requested in this one.

I'm not exactly sure if that is a bug, but it is surprising to me so I thought I would report it. I maintain a variety of software packages and like being able to switch back and forth between different build scenarios for development and debugging. I would like to use the --reuse option to avoid piling up so many packages when I switch back and forth between development efforts, but I can't because it pollutes the build environment when I do that.

Error message

There is no error. I was just surprised that spack would reuse packages with differing build options.

Information on your system

* **Spack:** 0.17.1-1200-c987d06a19
* **Python:** 3.9.7
* **Platform:** linux-ubuntu21.10-skylake
* **Concretizer:** clingo

General information

[X] I have run spack debug report and reported the version of Spack/Python/Platform
[X] I have searched the issues of this repo and believe this is not a duplicate
[X] I have run the failing commands in debug mode and reported the output

alalazo commented 2 years ago

This seems an interesting use case. We don't analyze compiler flags to automatically check for conflicts in them, and doing so is probably a daunting task. What we might be able to do though is to let the users customize, e.g. in packages.yaml, the directives being present in the packages by extending them.

The idea is that, if you could express in your environment that each package needs to be augmented with:

conflicts('cflags="-fsanitize=address -fno-omit-frame-pointer -g -Wall"')

you'll be able to solve the issues with --reuse.

@tgamblin @becker33 Do you have any thoughts on this?

tgamblin commented 2 years ago

Two things:

I think we at least need an option to reuse only from the current environment. Global reuse isn't always what you want. It may be that we want only-from-current-env to be the default for environments. This lines up with some of the discussion in #28468.
I actually don't have any problem with adding (gradually) constraints for particular compiler flags. I don't think we have to do it for all of them but we can pretty easily make it possible for folks to contribute conflicts in, say, a separate .lp file next to concretize.lp. We've already talked about adding more compatibility rules for OS's.

We likely need flag rules anyway -- to manage how linking is done for particular packages, particularly for compiler interop. So I'm actually not opposed to doing this the "right "way - I just don't think it has to happen all at once.

I'm not sure it's a bug if you ask for global reuse and get the previously compiled json-c with address sanitizer options, though I can see why you would want that particular flag to omit an install from consideration. I think it's probably more of a bug if something depends on json-c and reuses a sanitized build accidentally. You probably want some constraints like:

:- depends_on(Package, Dependency, "link"),
   node_flag(Package, "-fsanitize=address"), not node_flag(Dependency, "-fsanitize=address").
:- depends_on(Package, Dependency, "link"),
   not node_flag(Package, "-fsanitize=address"), node_flag(Dependency, "-fsanitize=address").

So that the solver can't pick the sanitized binary if the root isn't sanitized, or vice versa. Or, really, we should generalize that and just have certain flags that need to be together for compatibility:

:- depends_on(Package, Dependency, "link"), synced_flag(Flag),
   node_flag(Package, Flag), not node_flag(Dependency, Flag).
:- depends_on(Package, Dependency, "link"), synced_flag(Flag),
   not node_flag(Package, Flag), node_flag(Dependency, Flag).
synced_flag("-fsanitize=address").
% etc.

alalazo commented 2 years ago

@tgamblin Slightly OT but for 1. look at https://github.com/spack/spack/pull/28941 which changed the reuse argument to accept a list of specs instead of being just a boolean:

https://github.com/spack/spack/blob/253a6abc483c3b581e882220246f0f3ae826585c/lib/spack/spack/solver/asp.py#L2084-L2089

With that API change it should be easy to pick any set of specs we want to be reused into a solve.

carns commented 2 years ago

Thanks for considering this.

FWIW, I actually would like the ability to reuse packages across environments (as long as the full spec, including flags, variants, etc. matches exactly).

My use case is maybe a little odd. I have many spack environments that I use on my laptop. I help to maintain several packages in a dependency chain, for example x->y->z. If I'm working on z, then I activate an environment that has y as a root spec so that I can compile/develop/debug z by hand. If I'm working on y, then I activate an environment that has x as a root spec so that I can compile/develop/debug y by hand. Both of them may as well use the same x. In practice they probably share more like a dozen common dependencies.

That much works fine. The problem that prompted this issue report is that rather than having one environment to work on z in the above example, I actually have at least two. One of them may be called "z-dev" and has all sorts of debugging variants, cflags, etc. set that help me write correct code. One of them may be called "z-fast" and is built for speed with no debugging variants or sanitizers. I switch over to the latter to sanity check microbenchmarks to make sure the code is fast, and not just correct.

So for this use case I would like cross-environment reuse (because I have many environments that might share dozens of identical dependencies) yet I need them to not mix and match explicit cflags and variants because at any given time I really need my entire collection of packages to be built either all for debugging or all for speed. It really ends up being worse than just 2 categories of packages (debug or speed) because I might use multiple debugging tools that are incompatible and thus end up with multiple dev environments. As a concrete example, address sanitizer and valgrind do not get along, but neither covers a proper subset of the checks that the other does, so sometimes you need both to track down a problem. So then you have one build with address sanitizer, one with debugging symbols and some debugging variants, and one built for speed.

I think for the time being my work flow the safest thing is for me to avoid --reuse, accept that I'm going to have some duplicate packages installed, and use spack gc to clean things up when it seems like it's getting out of hand. If I had some kind of flag to set that told --reuse to not just consider the package name and version, but the full spec precisely, then that would cover my use case. I don't actually want to try to figure out which subsets of flags are compatible per se, I would be fine just knowing if they are an exact match or not.

spack / spack