Closed
Conversation
aidanheerdegen
force-pushed
the
aidanheerdegen/issue58
branch
from
24f8ca2 to
c3aec10
Compare
Member
Author
|
Currently the dual component development build errors: There are similar errors for the |
Member
Author
|
Another problem I came across: the instructions only seem to work for references like |
Member
Author
Now I think about it, maybe that is because I still had a Which brings up another point. Should we provide a |
CodeGat
requested changes
May 1, 2024
Member
CodeGat
left a comment
CodeGat
left a comment
There was a problem hiding this comment.
Seems good to me so far. Some small changes.
Co-authored-by: Tommy Gatti <[email protected]>
Member
Author
Thanks for the review. What do you think structurally? Covers the right ground? Too much detail? Does there need to be a TL;DR at the top that just says how to get going super quickly? Is it worth including all the Any opinion on providing a |
CodeGat
reviewed
May 2, 2024
Member
|
TLDRs are always good. On some of the logging. For example, a lot of the |
aekiss
reviewed
May 2, 2024
aekiss
reviewed
May 2, 2024
Contributor
|
This is looking great, thanks guys. Could this document also state explicitly where to find the newly-compiled executable and how to add it to config.yaml so it can be run? (Does anything else need to change in config.yaml?) |
harshula
reviewed
May 2, 2024
harshula
reviewed
May 2, 2024
Co-authored-by: Andrew Kiss <[email protected]> Co-authored-by: Tommy Gatti <[email protected]>
harshula
reviewed
May 7, 2024
harshula
reviewed
May 7, 2024
| 2. Concretize the updated environment. This will update what spack thinks is required to build the defined specs, in this case `access-om2` and `mom5`. Note that the `mom5` spec has a `dev_path` argument that points to the location of the sources it will use to build the package. | ||
|
|
||
| ```bash | ||
| $ spack concretize -f |
Collaborator
There was a problem hiding this comment.
==> Error: concretization failed for the following reasons:
1. Cannot select a single "version" for package "mom5"
2. Cannot satisfy '[email protected]=2023.11.09'
3. Cannot satisfy '[email protected]=2023.11.09'
4. Cannot satisfy '[email protected]=2023.11.09'
Collaborator
There was a problem hiding this comment.
I also received this. @harshula's suggestion to use
spack develop [email protected]
also fixed things for me (both with and without adding the spec).
harshula
reviewed
May 7, 2024
harshula
reviewed
May 7, 2024
Comment on lines
+100
to
+101
| $ spack add [email protected] | ||
| ==> Adding [email protected]=2023.11.09 to environment /g/data/.../dev_instructions/envs/mom5-dev |
Collaborator
There was a problem hiding this comment.
Double check if you need to add a package, since it is already included in the spack.yaml and you have added it with a version via develop.
Collaborator
|
The following worked: |
dougiesquire
previously approved these changes
May 7, 2024
Collaborator
dougiesquire
left a comment
dougiesquire
left a comment
There was a problem hiding this comment.
Thanks for doing this @aidanheerdegen! A couple of very minor comments to take or leave
| 2. Concretize the updated environment. This will update what spack thinks is required to build the defined specs, in this case `access-om2` and `mom5`. Note that the `mom5` spec has a `dev_path` argument that points to the location of the sources it will use to build the package. | ||
|
|
||
| ```bash | ||
| $ spack concretize -f |
Collaborator
There was a problem hiding this comment.
I also received this. @harshula's suggestion to use
spack develop [email protected]
also fixed things for me (both with and without adding the spec).
Collaborator
|
I was able to successfully change the version of a dependency. e.g. Change the version of
|
CodeGat
previously approved these changes
May 9, 2024
Member
Author
|
Thanks for the thorough review and feedback
I got an error at concretisation: I am chaining the If I delete that upstream it concretises ok. I guess that is due to a hash mismatches between Installation works, but is slow because it has to compile all the dependencies. So I think I've established that we can't mix |
Member
Author
Thanks again for working through this. I can confirm that this installed Tried But it didn't install a new version of Adding parallelio:
require: '@2.5.10'to the |
Co-authored-by: Dougie Squire <[email protected]>
ccarouge
reviewed
May 15, 2024
Member
ccarouge
left a comment
ccarouge
left a comment
There was a problem hiding this comment.
I was reading through the developer docs and I decided to put in my own comments. They are on the form only, since I'm reading through to learn the content. I haven't finished reading through but sending what I have already so it doesn't get lost.
Feel free to ignore if it isn't the right time for these.
Co-authored-by: Claire Carouge <[email protected]>
Member
Author
|
It was becoming unwieldy trying to investigate the issues with multi-component and updating dependencies, so I've turned this into a draft so it isn't merged, but we retain all the review comments in place. I've created a new PR: #67 to get a simpler version merged and work on the other examples individually. |
This was referenced May 23, 2024
Merged
Collaborator
|
Hi @aidanheerdegen , There's no |
Collaborator
First try doing |
Collaborator
|
Hi @aidanheerdegen , I was able to successfully change the version of a dependency and rebuild the dependents. e.g. Change the version of
|
Collaborator
|
CORRECTION:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Initial version to address #58