Skip to content

Parse % as ^ in specs #49808

Merged
tgamblin merged 8 commits intospack:developfrom
alalazo:parser/percent-parsed-as-caret
May 14, 2025
Merged

Parse % as ^ in specs #49808
tgamblin merged 8 commits intospack:developfrom
alalazo:parser/percent-parsed-as-caret

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented Mar 31, 2025

This PR modifies the parser, so that % is parsed as a DEPENDENCY, and all node properties that follow are associated to the name after the %. e.g., in foo %gcc +binutils the +binutils refers to gcc and not to foo.

% is still parsed as a build-type dependency, at the moment.

Environments, config files and package.py files from before Spack v1.0 may have spec strings with package variants, targets, etc. after a build dependency, and these will need to be updated. You can use the spack style --spec-strings command to do this.

To see what strings will be parsed differently under Spack v1.0, run:

spack style --spec-strings FILES

where FILES is a list of filenames that may contain old specs. To update these spec strings so that they parse correctly under both Spack 1.0 and Spack 0.x, you can run:

spack style --fix --spec-strings FILES

In the example above, foo %gcc +binutils would be rewritten as foo +binutils %gcc,
which parses the same in any Spack version.

In addition, this PR fixes several issues with % dependencies:

  • Ensure we can still constrain compilers on reuse
  • Ensure we can reuse a compiler by hash
  • Add tests

@spackbot-app spackbot-app bot added commands core PR affects Spack core functionality shell-support tests General test capability(ies) labels Mar 31, 2025
@alalazo alalazo changed the title Parse "%" as "^" in specs Parse % as ^ in specs Mar 31, 2025
@alalazo alalazo force-pushed the parser/percent-parsed-as-caret branch from fa9ddfe to 6ee75db Compare April 21, 2025 16:55
@alalazo alalazo force-pushed the parser/percent-parsed-as-caret branch from 6ee75db to dc8399b Compare April 29, 2025 07:35
@alalazo alalazo marked this pull request as ready for review April 29, 2025 07:36
@alalazo alalazo force-pushed the parser/percent-parsed-as-caret branch from 4016765 to 1db90b5 Compare April 29, 2025 08:44
@tgamblin tgamblin self-assigned this May 3, 2025
@alalazo alalazo force-pushed the parser/percent-parsed-as-caret branch 3 times, most recently from 4252e03 to bb382cf Compare May 6, 2025 10:33
@alalazo alalazo requested a review from tgamblin May 6, 2025 13:22
@alalazo alalazo force-pushed the parser/percent-parsed-as-caret branch 2 times, most recently from a659b61 to 93e4e05 Compare May 9, 2025 16:42
alalazo added 6 commits May 10, 2025 09:40
This commit modifies the parser, so that "%" is
parsed as a DEPENDENCY, and all node properties
that follow are associated to the name after the
"%", e.g. in "foo %gcc +binutils" the "+binutils"
refers to `gcc` and not to `foo` anymore.

The "%" is still parsed as a build-only dependency.

Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Massimiliano Culpo <[email protected]>
@alalazo alalazo force-pushed the parser/percent-parsed-as-caret branch from 93e4e05 to 6e37a54 Compare May 10, 2025 08:06
Copy link
Copy Markdown
Member

@tgamblin tgamblin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks mostly good -- a couple change requests.

alalazo added 2 commits May 13, 2025 11:23
Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Massimiliano Culpo <[email protected]>
@haampie
Copy link
Copy Markdown
Member

haampie commented May 14, 2025

I've updated the top-level description to mention spack style --spec-strings, which should be part of the commit message in case people bisect it.

@tgamblin tgamblin merged commit f8538a1 into spack:develop May 14, 2025
36 checks passed
@tgamblin tgamblin added this to the v1.0.0 milestone May 14, 2025
climbfuji pushed a commit to climbfuji/spack that referenced this pull request May 15, 2025
This PR modifies the parser, so that `%` is parsed as a `DEPENDENCY`, and all
node properties that follow are associated to the name after the `%`. e.g.,
in `foo %gcc +binutils` the `+binutils` refers to `gcc` and not to `foo`.

`%` is still parsed as a build-type dependency, at the moment.

Environments, config files and `package.py` files from before Spack v1.0 may have
spec strings with package variants, targets, etc. *after* a build dependency, and these
will need to be updated. You can use the `spack style --spec-strings` command to do this.

To see what strings will be parsed differently under Spack v1.0, run:

```
spack style --spec-strings FILES
```

where `FILES` is a list of filenames that may contain old specs. To update these spec
strings so that they parse correctly under both Spack 1.0 and Spack 0.x, you can run:

```
spack style --fix --spec-strings FILES
```

In the example above, `foo %gcc +binutils` would be rewritten as `foo +binutils %gcc`,
which parses the same in any Spack version.

In addition, this PR fixes several issues with `%` dependencies:

- [x] Ensure we can still constrain compilers on reuse
- [x] Ensure we can reuse a compiler by hash
- [x] Add tests

---------

Signed-off-by: Massimiliano Culpo <[email protected]>
@alalazo alalazo deleted the parser/percent-parsed-as-caret branch May 15, 2025 05:18
jezwilkinson pushed a commit to jezwilkinson/spack that referenced this pull request May 20, 2025
This PR modifies the parser, so that `%` is parsed as a `DEPENDENCY`, and all
node properties that follow are associated to the name after the `%`. e.g.,
in `foo %gcc +binutils` the `+binutils` refers to `gcc` and not to `foo`.

`%` is still parsed as a build-type dependency, at the moment.

Environments, config files and `package.py` files from before Spack v1.0 may have
spec strings with package variants, targets, etc. *after* a build dependency, and these
will need to be updated. You can use the `spack style --spec-strings` command to do this.

To see what strings will be parsed differently under Spack v1.0, run:

```
spack style --spec-strings FILES
```

where `FILES` is a list of filenames that may contain old specs. To update these spec
strings so that they parse correctly under both Spack 1.0 and Spack 0.x, you can run:

```
spack style --fix --spec-strings FILES
```

In the example above, `foo %gcc +binutils` would be rewritten as `foo +binutils %gcc`,
which parses the same in any Spack version.

In addition, this PR fixes several issues with `%` dependencies:

- [x] Ensure we can still constrain compilers on reuse
- [x] Ensure we can reuse a compiler by hash
- [x] Add tests

---------

Signed-off-by: Massimiliano Culpo <[email protected]>
abhishek1297 pushed a commit to abhishek1297/spack-melissa that referenced this pull request May 22, 2025
This PR modifies the parser, so that `%` is parsed as a `DEPENDENCY`, and all
node properties that follow are associated to the name after the `%`. e.g.,
in `foo %gcc +binutils` the `+binutils` refers to `gcc` and not to `foo`.

`%` is still parsed as a build-type dependency, at the moment.

Environments, config files and `package.py` files from before Spack v1.0 may have
spec strings with package variants, targets, etc. *after* a build dependency, and these
will need to be updated. You can use the `spack style --spec-strings` command to do this.

To see what strings will be parsed differently under Spack v1.0, run:

```
spack style --spec-strings FILES
```

where `FILES` is a list of filenames that may contain old specs. To update these spec
strings so that they parse correctly under both Spack 1.0 and Spack 0.x, you can run:

```
spack style --fix --spec-strings FILES
```

In the example above, `foo %gcc +binutils` would be rewritten as `foo +binutils %gcc`,
which parses the same in any Spack version.

In addition, this PR fixes several issues with `%` dependencies:

- [x] Ensure we can still constrain compilers on reuse
- [x] Ensure we can reuse a compiler by hash
- [x] Add tests

---------

Signed-off-by: Massimiliano Culpo <[email protected]>
alalazo added a commit that referenced this pull request May 29, 2025
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.

Signed-off-by: Massimiliano Culpo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commands core PR affects Spack core functionality new-variant shell-support tests General test capability(ies)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants