Synchronized env update for install #14621
Conversation
…t; this removes the db read tx entirely for the env update since it doesn't help much
…ifficult because unlike Environment.install it doesn't concretize anything or otherwise change the underlying .lock/.yaml file)
|
@tldahlgren @tgamblin this is a work in progress. I expect it will be improved by the end of today. It should be usable now (at least for making #13100 work with environments) |
…g a write transaction; refactor redundant logic between _install and install_all
…logic that re-reads the environment's state from the filesystem for the transaction
… written before the package is actually installed now
…reate a deadlock for multiple processes trying to install at the same time
…ns along with 'spack install' will not result in an inconsistent state
|
Closing/reopening to redo python version check |
…ocking when all specs are already installed
| def install_all(self, args=None): | ||
| """Install all concretized specs in an environment. | ||
|
|
||
| Note: this does not regenerate the views for the environment; | ||
| that needs to be done separately with a call to write(). | ||
|
|
||
| """ | ||
|
|
||
| # If "spack install" is invoked repeatedly for a large environment |
There was a problem hiding this comment.
Glad to see this comment.
…mpts to install specs that it found were uninstalled
|
@tgamblin @tldahlgren docstrings are added and the other requests are now addressed at this point too IMO |
There was a problem hiding this comment.
@scheibelp: one more request: I think that uninstall also needs a write transaction around the parts that update the environment. Can you add that?
|
On second thought, let's do uninstall in a separate PR. @scheibelp: can you follow up with that? |
Yes: I was looking into taking care of that and I'll open up a PR for it by mid-day tomorrow (1/29) |
|
@scheibelp: thanks! |
|
@tldahlgren: FYI, this is merged |
Great! I'll do another update after I test a change I just made. |
Just pushed the merge. |
This is a minimal set of edits to get parallel installs as implemented in #13100 working with environments.
What it currently does:
Environment.write_transactionEnvironment.write_transactionincmd.install(UPDATE: now passing)
One test is currently failing, which I think is an issue with the testing framwork vs. the actual logic (since I cannot manually reproduce the error when manually running through commands executed in the test).TODOs
spack add,spack rmetc.; this isn't strictly necessary for correcting parallel Spack installs but it should be done (perhaps later)Database.write_transactiontoEnvironment.write_transaction, which would include re-reading the environment (this would possibly allow moving the transaction logic fromcmd.installtoEnvironment