Skip to content

Do not attach dependency types to % by default#50565

Merged
alalazo merged 11 commits intospack:developfrom
alalazo:solver/direct-deps-only
May 29, 2025
Merged

Do not attach dependency types to % by default#50565
alalazo merged 11 commits intospack:developfrom
alalazo:solver/direct-deps-only

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented May 20, 2025 •
edited
Loading

Following #49808 this PR removes the default dependency type attached to the % sigil, so that ^ and % have a symmetric behavior.

Modifications:

  • Modify Spec.satisfies to account for different deptypes attached to %
  • Modify the solver to avoid assuming % refers to a "build only" deptype
  • Allow solving %[deptypes=link] mpich etc.

@spackbot-app spackbot-app bot added core PR affects Spack core functionality directives tests General test capability(ies) labels May 20, 2025
@alalazo alalazo added this to the v1.0.0 milestone May 20, 2025
@alalazo alalazo force-pushed the solver/direct-deps-only branch from bef870c to 1f9c16f Compare May 21, 2025 12:49
@alalazo alalazo mentioned this pull request May 21, 2025
Merged
2 tasks
@alalazo alalazo force-pushed the solver/direct-deps-only branch from 1f9c16f to 32454a4 Compare May 23, 2025 18:52
alalazo added 10 commits May 26, 2025 11:24
@alalazo
Branch based on the direct attribute of edges
4101f15 
Signed-off-by: Massimiliano Culpo <[email protected]>
@alalazo
Remove spurious comment in the parser
18ff940 
Signed-off-by: Massimiliano Culpo <[email protected]>
@alalazo
Parse % without a default depflag of 0
dc8b961 
Signed-off-by: Massimiliano Culpo <[email protected]>
@alalazo
Add dependency type to injected rules
57ba75c 
Signed-off-by: Massimiliano Culpo <[email protected]>
@alalazo
Emit "node_requirement" and derive "build_requirement"
d63de08 
Signed-off-by: Massimiliano Culpo <[email protected]>
@alalazo
Rework build requirement rules
0e7d3b4 
Instead of deducing build_requirement from a choice
on dependencies, do the opposite.

Signed-off-by: Massimiliano Culpo <[email protected]>
@alalazo
Use "node_requirement" instead of "build_requirement"
e91f8a4 
Signed-off-by: Massimiliano Culpo <[email protected]>
@alalazo
Add [deptypes=build] to injected libc and compiler-wrapper
c5255f5 
Signed-off-by: Massimiliano Culpo <[email protected]>
@alalazo
Do not assume deptype "build" when % is used
4b68412 
Signed-off-by: Massimiliano Culpo <[email protected]>
@alalazo
Fixup after rebase
03d10e4 
Signed-off-by: Massimiliano Culpo <[email protected]>
@alalazo alalazo force-pushed the solver/direct-deps-only branch 2 times, most recently from 38a88e8 to 6b43e35 Compare May 26, 2025 15:37
@alalazo
Allow using %[deptypes=link] <package>
2c91352 
Signed-off-by: Massimiliano Culpo <[email protected]>
@alalazo alalazo force-pushed the solver/direct-deps-only branch from 6b43e35 to 2c91352 Compare May 26, 2025 16:43
@alalazo alalazo marked this pull request as ready for review May 26, 2025 17:05
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented May 26, 2025

This is kind of unexpected, but it seems this change in modeling didn't come with any slowdown. Benchmarked against:

  • Spack: 1.0.0.dev0 (0f1f438)
  • Python: 3.13.0
  • Platform: linux-ubuntu20.04-icelake

radiuss.develop.csv
radiuss.pr.csv

radiuss

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented May 27, 2025

@spackbot run pipeline

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented May 27, 2025

I've started that pipeline for you!

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented May 29, 2025

Failures in pipelines are not related:

@alalazo alalazo merged commit e4d4f6c into spack:develop May 29, 2025
33 of 34 checks passed
@alalazo alalazo deleted the solver/direct-deps-only branch May 29, 2025 08:02
@alecbcs
Copy link
Copy Markdown
Member

alecbcs commented May 31, 2025

@alalazo after this PR/commit I can no longer concretize emacs +native. I've checked my packages configuration and the gcc external correctly shows as having languages=jit so I'm a bit at a loss for why the package is suddenly failing to use the external.

> git checkout 3f33c717487e5f1ed3427e3966721dbb6839af69
alecbcs@laptop:~/s/s/spack ((HEAD detached at 3f33c7174))
> spack spec --fresh emacs+native                      
 -   [email protected]~json+native+tls~treesitter build_system=autotools gui=none arch=linux-debian13-x86_64_v4
 -       ^[email protected] build_system=generic arch=linux-debian13-x86_64_v4
[e]      ^[email protected]~binutils+bootstrap~graphite~mold~nvptx~piclibs~profiled+strip build_system=autotools build_type=RelWithDebInfo languages:='c,c++,fortran,jit' arch=linux-debian13-x86_64_v4
 -       ^[email protected] build_system=generic arch=linux-debian13-x86_64_v4
[e]      ^[email protected] build_system=autotools arch=linux-debian13-x86_64_v4
 -       ^[email protected]~guile build_system=generic arch=linux-debian13-x86_64_v4
 -       ^[email protected]+brotli+zlib+zstd build_system=autotools arch=linux-debian13-x86_64_v4
 -           ^[email protected]~ipo build_system=cmake build_type=Release generator=make arch=linux-debian13-x86_64_v4
 -           ^[email protected]+bzip2+curses+git~libunistring+libxml2+pic+shared+tar+xz build_system=autotools arch=linux-debian13-x86_64_v4
 -               ^[email protected]~debug~pic+shared build_system=generic arch=linux-debian13-x86_64_v4
 -                   ^[email protected] build_system=autotools arch=linux-debian13-x86_64_v4
 -               ^[email protected] build_system=autotools zip=pigz arch=linux-debian13-x86_64_v4
 -                   ^[email protected] build_system=makefile arch=linux-debian13-x86_64_v4
 -           ^[email protected] build_system=autotools arch=linux-debian13-x86_64_v4
 -               ^[email protected] build_system=autotools arch=linux-debian13-x86_64_v4
 -           ^[email protected] build_system=autotools arch=linux-debian13-x86_64_v4
 -               ^[email protected] build_system=autotools patches:=440b954 arch=linux-debian13-x86_64_v4
 -               ^[email protected]+sigsegv build_system=autotools patches:=9dc5fbd,bfdffa7 arch=linux-debian13-x86_64_v4
 -                   ^[email protected] build_system=autotools arch=linux-debian13-x86_64_v4
 -           ^[email protected] build_system=autotools arch=linux-debian13-x86_64_v4
 -               ^[email protected]+cxx build_system=autotools libs:=shared,static arch=linux-debian13-x86_64_v4
 -                   ^[email protected] build_system=autotools arch=linux-debian13-x86_64_v4
 -                   ^[email protected] build_system=autotools arch=linux-debian13-x86_64_v4
 -           ^[email protected]+programs build_system=makefile compression:=none libs:=shared,static arch=linux-debian13-x86_64_v4
 -       ^[email protected] build_system=autotools arch=linux-debian13-x86_64_v4
 -       ^[email protected]~ipo~jpeg8~partial_decoder+pic build_system=cmake build_type=Release generator=make libs:=shared,static arch=linux-debian13-x86_64_v4
 -           ^[email protected]~doc+ncurses+ownlibs~qtgui build_system=generic build_type=Release arch=linux-debian13-x86_64_v4
 -               ^[email protected]~gssapi~ldap~libidn2~librtmp~libssh~libssh2+nghttp2 build_system=autotools libs:=shared,static tls:=openssl arch=linux-debian13-x86_64_v4
 -                   ^[email protected] build_system=autotools arch=linux-debian13-x86_64_v4
 -                   ^[email protected]~docs+shared build_system=generic certs=mozilla arch=linux-debian13-x86_64_v4
 -                       ^ca-certificates-mozilla@2025-02-25 build_system=generic arch=linux-debian13-x86_64_v4
 -           ^[email protected] build_system=autotools arch=linux-debian13-x86_64_v4
 -       ^[email protected]~http+pic~python+shared build_system=autotools arch=linux-debian13-x86_64_v4
 -           ^[email protected] build_system=autotools libs:=shared,static arch=linux-debian13-x86_64_v4
 -           ^[email protected]~pic build_system=autotools libs:=shared,static arch=linux-debian13-x86_64_v4
 -       ^[email protected]~symlinks+termlib abi=none build_system=autotools patches:=7a351bc arch=linux-debian13-x86_64_v4
 -       ^[email protected]~jit+multibyte+pic+shared+static+utf build_system=autotools arch=linux-debian13-x86_64_v4
 -       ^[email protected] build_system=autotools arch=linux-debian13-x86_64_v4
 -       ^[email protected] build_system=autotools arch=linux-debian13-x86_64_v4
 -           ^[email protected]+cpanm+opcode+open+shared+threads build_system=generic arch=linux-debian13-x86_64_v4
 -               ^[email protected]+cxx~docs+stl build_system=autotools patches:=26090f4,b231fcc arch=linux-debian13-x86_64_v4
 -               ^[email protected] build_system=autotools arch=linux-debian13-x86_64_v4
 -                   ^[email protected] build_system=autotools patches:=1ea4349,24f587b,3d9885e,5911a5b,622ba38,6c8adf8,758e2ec,79572ee,a177edc,bbf97f1,c7b45ff,e0013d9,e065038 arch=linux-debian13-x86_64_v4
 -       ^[email protected]+compat+new_strategies+opt+pic+shared build_system=autotools arch=linux-debian13-x86_64_v4


alecbcs@laptop:~/s/s/spack ((HEAD detached at 3f33c7174))
> git checkout e4d4f6cbc8b4458dcc480224b30dd5d250aa0174
Previous HEAD position was 3f33c7174 NCEPLIBS recipe updates (#50257)
HEAD is now at e4d4f6cbc Do not attach dependency types to `%` by default (#50565)

alecbcs@laptop:~/s/s/spack ((HEAD detached at e4d4f6cbc))
> spack spec --fresh emacs+native                      
==> Error: failed to concretize `emacs+native` for the following reasons:
     1. Only external, or concrete, compilers are allowed for the c language

@alecbcs alecbcs mentioned this pull request May 31, 2025
Closed
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented May 31, 2025

@alecbcs I'll have a look asap next week. It's likely some edge case missing for externals.

alalazo added a commit to alalazo/spack that referenced this pull request Jun 1, 2025
@alalazo
Fix allowing conditional link dependencies on a compiler
8cab6fa 
Also restores internal_error, which was unintentionally
dropped in spack#50565

Signed-off-by: Massimiliano Culpo <[email protected]>
@alalazo alalazo mentioned this pull request Jun 1, 2025
Merged
alecbcs pushed a commit that referenced this pull request Jun 1, 2025
@alalazo
Fix allowing conditional link dependencies on a compiler (#50738)
674f8c0 
Also restores internal_error, which was unintentionally
dropped in #50565

Signed-off-by: Massimiliano Culpo <[email protected]>
kshea21 pushed a commit to kshea21/spack that referenced this pull request Jun 18, 2025
@alalazo @kshea21
Fix allowing conditional link dependencies on a compiler (spack#50738)
a2254b8 
Also restores internal_error, which was unintentionally
dropped in spack#50565

Signed-off-by: Massimiliano Culpo <[email protected]>
alalazo added a commit to alalazo/spack that referenced this pull request Jul 30, 2025
@alalazo
solver: fix issue when llvm is not a pure build dependency
db691c5 
fixes spack#51058

spack#50565 regressed this case by missing the fact that
a "provider_set" on the direct dependency must be
honored even if the direct dependency turns out to
be needed at runtime

Signed-off-by: Massimiliano Culpo <[email protected]>
@alalazo alalazo mentioned this pull request Jul 30, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tgamblin tgamblin tgamblin approved these changes

Assignees

No one assigned

Labels

core PR affects Spack core functionality directives intel tests General test capability(ies) update-package

Projects

None yet

Milestone

v1.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@alalazo @alecbcs @tgamblin