Age | Commit message (Collapse) | Author | Files | Lines |
|
|
|
## Motivation
Our parser grew to be quite complex, with a 2-state lexer and logic in the parser
that has up to 5 levels of nested conditionals. In the future, to turn compilers into
proper dependencies, we'll have to increase the complexity further as we foresee
the need to add:
1. Edge attributes
2. Spec nesting
to the spec syntax (see https://github.com/spack/seps/pull/5 for an initial discussion of
those changes). The main attempt here is thus to _simplify the existing code_ before
we start extending it later. We try to do that by adopting a different token granularity,
and by using more complex regexes for tokenization. This allow us to a have a "flatter"
encoding for the parser. i.e., it has fewer nested conditionals and a near-trivial lexer.
There are places, namely in `VERSION`, where we have to use negative lookahead judiciously
to avoid ambiguity. Specifically, this parse is ambiguous without `(?!\s*=)` in `VERSION_RANGE`
and an extra final `\b` in `VERSION`:
```
@ 1.2.3 : develop # This is a version range 1.2.3:develop
@ 1.2.3 : develop=foo # This is a version range 1.2.3: followed by a key-value pair
```
## Differences with the previous parser
~There are currently 2 known differences with the previous parser, which have been added on purpose:~
- ~No spaces allowed after a sigil (e.g. `foo @ 1.2.3` is invalid while `foo @1.2.3` is valid)~
- ~`/<hash> @1.2.3` can be parsed as a concrete spec followed by an anonymous spec (before was invalid)~
~We can recover the previous behavior on both ones but, especially for the second one, it seems the current behavior in the PR is more consistent.~
The parser is currently 100% backward compatible.
## Error handling
Being based on more complex regexes, we can possibly improve error
handling by adding regexes for common issues and hint users on that.
I'll leave that for a following PR, but there's a stub for this approach in the PR.
## Performance
To be sure we don't add any performance penalty with this new encoding, I measured:
```console
$ spack python -m timeit -s "import spack.spec" -c "spack.spec.Spec(<spec>)"
```
for different specs on my machine:
* **Spack:** 0.20.0.dev0 (c9db4e50ba045f5697816187accaf2451cb1aae7)
* **Python:** 3.8.10
* **Platform:** linux-ubuntu20.04-icelake
* **Concretizer:** clingo
results are:
| Spec | develop | this PR |
| ------------- | ------------- | ------- |
| `trilinos` | 28.9 usec | 13.1 usec |
| `trilinos @1.2.10:1.4.20,2.0.1` | 131 usec | 120 usec |
| `trilinos %gcc` | 44.9 usec | 20.9 usec |
| `trilinos +foo` | 44.1 usec | 21.3 usec |
| `trilinos foo=bar` | 59.5 usec | 25.6 usec |
| `trilinos foo=bar ^ mpich foo=baz` | 120 usec | 82.1 usec |
so this new parser seems to be consistently faster than the previous one.
## Modifications
In this PR we just substituted the Spec parser, which means:
- [x] Deleted in `spec.py` the `SpecParser` and `SpecLexer` classes. deleted `spack/parse.py`
- [x] Added a new parser in `spack/parser.py`
- [x] Hooked the new parser in all the places the previous one was used
- [x] Adapted unit tests in `test/spec_syntax.py`
## Possible future improvements
Random thoughts while working on the PR:
- Currently we transform hashes and files into specs during parsing. I think
we might want to introduce an additional step and parse special objects like
a `FileSpec` etc. in-between parsing and concretization.
|
|
|
|
|
|
|
|
|
|
This commit reworks the bootstrapping procedure to use Spack environments
as much as possible.
The `spack.bootstrap` module has also been reorganized into a Python package.
A distinction is made among "core" Spack dependencies (clingo, GnuPG, patchelf)
and other dependencies. For a number of reasons, explained in the `spack.bootstrap.core`
module docstring, "core" dependencies are bootstrapped with the current ad-hoc
method.
All the other dependencies are instead bootstrapped using a Spack environment
that lives in a directory specific to the interpreter and the architecture being used.
|
|
All the vermin annotations we were using were for optional features introduced in early
Python 3 versions. We no longer need any of them, as we only support Python 3.6+. If we
start optionally using features from newer Pythons than 3.6, we will need more vermin
annotations.
Co-authored-by: Harmen Stoppels <harmenstoppels@gmail.com>
Co-authored-by: Harmen Stoppels <harmenstoppels@gmail.com>
|
|
We no longer support Python <3.6, so we don't need to check whether Python supports SSL
verification in `spack.util.web`.
- [x] Remove a bunch of logic we needed to appease Python 2
|
|
We've stopped supporting Python 2, and contributors are noticing that our CI no longer
allows Python 2.7 comment type hints. They end up having to adapt them, but this adds
extra unrelated work to PRs.
- [x] Move to 3.6 type hints across the entire code base
|
|
* patch command: add concretizer args
* tab completion
|
|
|
|
All Spec attributes are now represented as `attr(attribute_name, ... args ...)`, e.g.
`attr(node, "hdf5")` instead of `node("hdf5")`, as we *have* to maintain the `attr()`
form anyway, and it simplifies the encoding to just maintain one form of the Spec
information.
Background
----------
In #20644, we unified the way conditionals are done in the concretizer, but this
introduced a nasty aspect to the encoding: we have to maintain everything we want in
general conditions in two forms: `predicate(...)` and `attr("predicate", ...)`. For
example, here's the start of the table of spec attributes we had to maintain:
```prolog
node(Package) :- attr("node", Package).
virtual_node(Virtual) :- attr("virtual_node", Virtual).
hash(Package, Hash) :- attr("hash", Package, Hash).
version(Package, Version) :- attr("version", Package, Version).
...
```
```prolog
attr("node", Package) :- node(Package).
attr("virtual_node", Virtual) :- virtual_node(Virtual).
attr("hash", Package, Hash) :- hash(Package, Hash).
attr("version", Package, Version) :- version(Package, Version).
...
```
This adds cognitive load to understanding how the concretizer works, as you have to
understand the equivalence between the two forms of spec attributes. It also makes the
general condition logic in #20644 hard to explain, and it's easy to forget to add a new
equivalence to this list when adding new spec attributes (at least two people have been
bitten by this).
Solution
--------
- [x] remove the equivalence list from `concretize.lp`
- [x] simplify `spec_clauses()`, `condition()`, and other functions in `asp.py` that need
to deal with `Spec` attributes.
- [x] Convert all old-form spec attributes in `concretize.lp` to the `attr()` form
- [x] Simplify `display.lp`, where we also had to maintain a list of spec attributes. Now
we only need to show `attr/2`, `attr/3`, and `attr/4`.
- [x] Simplify model extraction logic in `asp.py`.
Performance
-----------
This seems to result in a smaller grounded problem (as there are no longer duplicated
`attr("foo", ...)` / `foo(...)` predicates in the program), but it also adds a slight
performance overhead vs. develop. Ultimately, simplifying the encoding will be a win,
particularly for improving error messages.
Notes
-----
This will simplify future node refactors in `concretize.lp` (e.g., not identifying nodes
by package name, which we need for separate build dependencies).
I'm still not entirely used to reading `attr()` notation, but I thnk it's ultimately
clearer than what we did before. We need more uniform naming, and it's now clear what is
part of a solution. We should probably continue making the encoding of `concretize.lp`
simpler and more self-explanatory. It may make sense to rename `attr` to something like
`node_attr` and to simplify the names of node attributes. It also might make sense to do
something similar for other types of predicates in `concretize.lp`.
|
|
* Bugfix: Fetch should not force use of curl to check url existence
* Switch type hints from comments to actual hints
|
|
|
|
|
|
|
|
(#34201)
We currently report that searched config paths don't exist at debug level 1, which
clutters the output quite a bit:
```console
> spack -d solve --fresh --show asp hdf5 > hdf5.lp
==> [2022-11-29-14:18:21.035133] Skipping nonexistent config path /Users/gamblin2/src/spack/etc/spack/darwin/concretizer.yaml
==> [2022-11-29-14:18:21.035151] Skipping nonexistent config path /Users/gamblin2/.spack/concretizer.yaml
==> [2022-11-29-14:18:21.035169] Skipping nonexistent config path /Users/gamblin2/.spack/darwin/concretizer.yaml
==> [2022-11-29-14:18:21.035238] Reading config from file /Users/gamblin2/src/spack/etc/spack/defaults/repos.yaml
==> [2022-11-29-14:18:21.035996] Skipping nonexistent config path /Users/gamblin2/src/spack/etc/spack/defaults/darwin/repos.yaml
==> [2022-11-29-14:18:21.036021] Skipping nonexistent config path /etc/spack/repos.yaml
==> [2022-11-29-14:18:21.036039] Skipping nonexistent config path /etc/spack/darwin/repos.yaml
==> [2022-11-29-14:18:21.036057] Skipping nonexistent config path /Users/gamblin2/src/spack/etc/spack/repos.yaml
==> [2022-11-29-14:18:21.036072] Skipping nonexistent config path /Users/gamblin2/src/spack/etc/spack/darwin/repos.yaml
==> [2022-11-29-14:18:21.036088] Skipping nonexistent config path /Users/gamblin2/.spack/repos.yaml
==> [2022-11-29-14:18:21.036105] Skipping nonexistent config path /Users/gamblin2/.spack/darwin/repos.yaml
==> [2022-11-29-14:18:21.071828] Reading config from file /Users/gamblin2/src/spack/etc/spack/defaults/config.yaml
==> [2022-11-29-14:18:21.081628] Skipping nonexistent config path /Users/gamblin2/src/spack/etc/spack/defaults/darwin/config.yaml
==> [2022-11-29-14:18:21.081669] Skipping nonexistent config path /etc/spack/config.yaml
==> [2022-11-29-14:18:21.081692] Skipping nonexistent config path /etc/spack/darwin/config.yaml
==> [2022-11-29-14:18:21.081712] Skipping nonexistent config path /Users/gamblin2/src/spack/etc/spack/config.yaml
==> [2022-11-29-14:18:21.081731] Skipping nonexistent config path /Users/gamblin2/src/spack/etc/spack/darwin/config.yaml
==> [2022-11-29-14:18:21.081748] Skipping nonexistent config path /Users/gamblin2/.spack/config.yaml
==> [2022-11-29-14:18:21.081764] Skipping nonexistent config path /Users/gamblin2/.spack/darwin/config.yaml
==> [2022-11-29-14:18:21.134909] Reading config from file /Users/gamblin2/src/spack/etc/spack/defaults/packages.yaml
==> [2022-11-29-14:18:21.148695] Reading config from file /Users/gamblin2/src/spack/etc/spack/defaults/darwin/packages.yaml
==> [2022-11-29-14:18:21.152555] Skipping nonexistent config path /etc/spack/packages.yaml
==> [2022-11-29-14:18:21.152582] Skipping nonexistent config path /etc/spack/darwin/packages.yaml
==> [2022-11-29-14:18:21.152601] Skipping nonexistent config path /Users/gamblin2/src/spack/etc/spack/packages.yaml
==> [2022-11-29-14:18:21.152620] Skipping nonexistent config path /Users/gamblin2/src/spack/etc/spack/darwin/packages.yaml
==> [2022-11-29-14:18:21.152637] Skipping nonexistent config path /Users/gamblin2/.spack/packages.yaml
==> [2022-11-29-14:18:21.152654] Skipping nonexistent config path /Users/gamblin2/.spack/darwin/packages.yaml
==> [2022-11-29-14:18:21.853915] Skipping nonexistent config path /Users/gamblin2/src/spack/etc/spack/defaults/compilers.yaml
==> [2022-11-29-14:18:21.853962] Skipping nonexistent config path /Users/gamblin2/src/spack/etc/spack/defaults/darwin/compilers.yaml
==> [2022-11-29-14:18:21.853987] Skipping nonexistent config path /etc/spack/compilers.yaml
==> [2022-11-29-14:18:21.854007] Skipping nonexistent config path /etc/spack/darwin/compilers.yaml
==> [2022-11-29-14:18:21.854025] Skipping nonexistent config path /Users/gamblin2/src/spack/etc/spack/compilers.yaml
==> [2022-11-29-14:18:21.854043] Skipping nonexistent config path /Users/gamblin2/src/spack/etc/spack/darwin/compilers.yaml
==> [2022-11-29-14:18:21.854060] Skipping nonexistent config path /Users/gamblin2/.spack/compilers.yaml
==> [2022-11-29-14:18:21.854093] Reading config from file /Users/gamblin2/.spack/darwin/compilers.yaml
```
It is very rare that I want to know this much information about config search, so I've
moved this to level 3. Now at level 1, we can see much more clearly what configs were
actually found:
```console
> spack -d solve --fresh --show asp hdf5 > hdf5.lp
==> [2022-11-29-14:19:04.035457] Imported solve from built-in commands
==> [2022-11-29-14:19:04.035818] Imported solve from built-in commands
==> [2022-11-29-14:19:04.037626] Reading config from file /Users/gamblin2/src/spack/etc/spack/defaults/concretizer.yaml
==> [2022-11-29-14:19:04.040033] Reading config from file /Users/gamblin2/src/spack/etc/spack/defaults/repos.yaml
==> [2022-11-29-14:19:04.080852] Reading config from file /Users/gamblin2/src/spack/etc/spack/defaults/config.yaml
==> [2022-11-29-14:19:04.133241] Reading config from file /Users/gamblin2/src/spack/etc/spack/defaults/packages.yaml
==> [2022-11-29-14:19:04.147175] Reading config from file /Users/gamblin2/src/spack/etc/spack/defaults/darwin/packages.yaml
==> [2022-11-29-14:19:05.157896] Reading config from file /Users/gamblin2/.spack/darwin/compilers.yaml
```
You can still get the old messages with `spack -ddd` (to activate debug level 3).
|
|
* warn about removal of deprecated format strings
Co-authored-by: becker33 <becker33@users.noreply.github.com>
|
|
env (#34059)
This fixes an issue introduced in #32340, which changed the semantics of the "module"
object passed to the "setup_dependent_package" callback.
|
|
binaries. (#33746)" (#34087)" (#34153)
This reverts commit 63e440651436fdc30993ef58875279e71f4851d1.
|
|
|
|
This reverts commit d06fd26c9ac8dd525fc129096188e2ea9fd2d0d7.
The problem is that Bitbucket's API forwards download requests to an S3 bucket using a temporary URL. This URL includes a signature for the request, which embeds the HTTP verb. That means only GET requests are allowed, and HEAD requests would fail verification, leading to 403 erros. The same is observed when using `curl -LI ...`
|
|
|
|
|
|
Using `-Werror` is good practice for development and testing, but causes us a great
deal of heartburn supporting multiple compiler versions, especially as newer compiler
versions add warnings for released packages. This PR adds support for suppressing
`-Werror` through spack's compiler wrappers. There are currently three modes for
the `flags:keep_werror` setting:
* `none`: (default) cancel all `-Werror`, `-Werror=*` and `-Werror-*` flags by
converting them to `-Wno-error[=]*` flags
* `specific`: preserve explicitly selected warnings as errors, such as
`-Werror=format-truncation`, but reverse the blanket `-Werror`
* `all`: keeps all `-Werror` flags
These can be set globally in config.yaml, through the config command-line flags, or
overridden by a particular package (some packages use Werror as a proxy for determining
support for other compiler features). We chose to use this approach because:
1. removing `-Werror` flags entirely broke *many* build systems, especially autoconf
based ones, because of things like checking `-Werror=feature` and making the
assumption that if that did not error other flags related to that feature would also work
2. Attempting to preserve `-Werror` in some phases but not others caused similar issues
3. The per-package setting came about because some packages, even with all these
protections, still use `-Werror` unsafely. Currently there are roughly 3 such packages
known.
|
|
For reasons beyond me Python thinks it's a great idea to upgrade HEAD
requests to GET requests when following redirects. So, this PR adds a
better `HTTPRedirectHandler`, and also moves some ad-hoc logic around
for dealing with disabling SSL certs verification.
Also, I'm stumped by the fact that Spack's `url_exists` does not use
HEAD requests at all, so in certain cases Spack awkwardly downloads
something first to see if it can download it, and then downloads it
again because it knows it can download it. So, this PR ensures that both
urllib and botocore use HEAD requests.
Finally, it also removes some things that were there to support currently
unsupported Python versions.
Notice that the HTTP spec [section 10.3.2](https://datatracker.ietf.org/doc/html/rfc2616.html#section-10.3.2) just talks about how to deal
with POST request on redirect (whether to follow or not):
> If the 301 status code is received in response to a request other
> than GET or HEAD, the user agent MUST NOT automatically redirect the
> request unless it can be confirmed by the user, since this might
> change the conditions under which the request was issued.
> Note: When automatically redirecting a POST request after
> receiving a 301 status code, some existing HTTP/1.0 user agents
> will erroneously change it into a GET request.
Python has a comment about this, they choose to go with the "erroneous change".
But they then mess up the HEAD request while following the redirect, probably
because they were too busy discussing how to deal with POST.
See https://github.com/python/cpython/pull/99731
|
|
(#33746)" (#34087)
This reverts commit 5c4137baf19b9e271a2f13e886d6b875aab067dd.
|
|
|
|
|
|
This adds super-lazy maintainer mode to `spack checksum`: Instead of
only printing the new checksums to the terminal, `-a` and
`--add-to-package` will add the new checksums to the `package.py` file
and open it in the editor afterwards for final checks.
|
|
|
|
This reverts commit 7f9af8d4a0bfbb1577e5ac9982624d8d0cb9b9ca.
|
|
Co-authored-by: becker33 <becker33@users.noreply.github.com>
|
|
* Add a WindowsRegistryView class, which can query for existing
package installations on Windows. This is particularly important
because some Windows packages (including those added here)
do not allow two simultaneous installs, and this can be
queried in order to provide a clear error message.
* Consolidate external path detection logic for Windows into
WindowsKitExternalPaths and WindowsCompilerExternalPaths objects.
* Add external-only packages win-sdk and wgl
* Add win-wdk (including external detection) which depends on
win-sdk
* Replace prior msmpi implementation with a source-based install
(depends on win-wdk). This install can control the install
destination (unlike the binary installation).
* Update MSVC compiler to choose vcvars based on win-sdk dependency
* Provide "msbuild" module-level variable to packages during build
* When creating symlinks on Windows, need to explicitly specify when
a symlink target is a directory
* executables_in_path no-longer defaults to using PATH (this is
now expected to be taken care of by the caller)
|
|
Spec traversals can now specify a topological ordering. A topologically-
ordered traversal with input specs X1, X2... will
* include all of X1, X2... and their children
* be ordered such that a given node is guaranteed to appear before any
of its children in the traversal
Other notes:
* Input specs can be children of other input specs (this is useful if
a user specifies a set of specs to uninstall: some of those specs
might be children of others)
* `direction="parents"` will produce a reversed topological order
(children always come before parents).
* `cover="edges"` will generate a list of edges L such that (a) input
edges will always appear before output edges and (b) if you create
a list with the destination of each edge in L the result is
topologically ordered
|
|
* test_suite.py: speed up slow test by using mock packages
* Don't resolve the sha during unit-tests
* Skip long-running test that fails, instead of executing it
|
|
|
|
* uninstall: fix accidental cubic complexity
Currently spack uninstall runs in worst case cubic time complexity
thanks to traversal during traversal during traversal while collecting
the specs to be uninstalled.
Also brings down the number of error messages printed to something
linear in the amount of matching specs instead of quadratic.
|
|
|
|
|
|
* Add a regression test for 33928
* PackageBase should not set `(build|install)_time_test_callbacks`
* Fix audits by preserving the current semantic
Co-authored-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>
|
|
refers #33985
|
|
* Enable hdf5 build (including +mpi) on Windows
* This includes updates to hdf5 dependencies openssl (minor edit) and
bzip2 (more-extensive edits)
* Add binary-based installation of msmpi (this is currently the only
supported MPI implementation in Spack for Windows). Note that this
does not install to the Spack-specified prefix. This implementation
will be replaced with a source-based implementation
Co-authored-by: John Parent <john.parent@kitware.com>
|
|
Fix erroneous duplication of `build_time_test_callbacks` in
`legacy_attributes`: one of the duplicates should be
`install_time_test_callbacks`
|
|
|
|
|
|
These commands are slated for removal in v0.20
|
|
|
|
|