Replies: 36 comments 58 replies
-
Given the set of DRAFT PRs submitted by @jedwards4b, I thought it would be good to have a single place where we could discuss the pros and cons of this new approach and any possible required changes if it is to be accepted. I wrote down a set of requirements as a strawman. I hope the community weighs in and that these are updated as a consensus emerges. |
Beta Was this translation helpful? Give feedback.
-
I hope you don't mind I edited your list so that I can refer to it by index. Issue 1: I believe this is fully covered. Please provide a counter example if you disagree. Issue 2 is a bit more difficult, git submodules use hashes and not tags, you can see the list of submodules in the toplevel .gitmodules file and you can see the list of hashes in github. One suggestion to solve this problem is to continue to use a file like Externals.cfg and add code to the install script to maintain the consistency of that file. See discussion here. Issue 3 currently the install scripts can either install internal only or internal plus external submodules - extending this to be able to control individual submodules should be a straightforward change. Issue 4 is fully covered by this change and in fact the proposed code does a better job of this than manage externals - in manage externals the full code is checked out and then pruned to be the requested sparse code. In the proposed change the pruning is done before the code is checked out and only the requested sparse checkout files are downloaded. Issue 5 I think this is covered, although I find git messages to be fairly obscure sometimes. Issue 6 I don't believe that there is a problem here, a counter example may help. Issue 7 Not currently handled but as with Issue 3 I don't think that it would be a difficult extension. |
Beta Was this translation helpful? Give feedback.
-
Regarding requirement 2 (see tags before checkout):
I have been thinking that this is a good solution. We can use the same format but drop the protocol field. I have not yet thought through how requirement 3 will be met. |
Beta Was this translation helpful? Give feedback.
-
Regarding requirement 6 (developer modifying code):
I think the biggest issue here is to define what behavior we want in all cases. Maybe it is as simple as having a |
Beta Was this translation helpful? Give feedback.
-
Two thoughts on this:
|
Beta Was this translation helpful? Give feedback.
-
I have something I would like to get feedback on. I've only updated cam so far so if you would like to try and send me feedback please run:
If you change tags in .gitmodules running ./esm --update will update the sandbox to the new tags. |
Beta Was this translation helpful? Give feedback.
-
Update: I've renamed the driver script git-fleximod and refactored a little. |
Beta Was this translation helpful? Give feedback.
-
The empty directories are a git submodules feature, I don't think I can do much about that.
I think maybe they get the git-fleximod repo independent of the model repo - what do you think? |
Beta Was this translation helpful? Give feedback.
-
So I've changed the --status and --update options so that they are positional and added install.
or git-fleximod if you don't want to put it in the PATH. |
Beta Was this translation helpful? Give feedback.
-
I was trying to give this a try, but when I |
Beta Was this translation helpful? Give feedback.
-
@cacraigucar It's been renamed to install again, so running ./install is correct. What error did you get? I just tried and it seems to be working. |
Beta Was this translation helpful? Give feedback.
-
Here is the error message: izumi$ ./install |
Beta Was this translation helpful? Give feedback.
-
That got me further, (I tried it twice) but it still bombs out: izumi$ module load lang/python/3.11.5 |
Beta Was this translation helpful? Give feedback.
-
git-fleximod is now available on pypi: https://pypi.org/project/fleximod/ |
Beta Was this translation helpful? Give feedback.
-
Following up on @mwaxmonsky 's suggestion in the CSEG meeting, has CMake been explored as a possible solution? There is built-in functionality like |
Beta Was this translation helpful? Give feedback.
-
and
Something that reflects its purpose.
I'm not sure it is an extension since it does not support actions such as |
Beta Was this translation helpful? Give feedback.
-
But I did include a suggestion. What am I missing here? |
Beta Was this translation helpful? Give feedback.
-
Again, that was not the intent, please let me know why you came to that conclusion. I was confused by the mismatch and wanted to make sure it was just the specific tags that were used in the example and not a "feature" of the tool. Thanks for clarifying. |
Beta Was this translation helpful? Give feedback.
-
I have updated CESM, CAM and CTSM branches with a new version of git-fleximod. Please give it another try. @gold2718 |
Beta Was this translation helpful? Give feedback.
-
Updated to add support for fxhash. My feeling is that I should add this to the github workflow testing and fail if a PR is going to a protected branch with an fxhash reference. |
Beta Was this translation helpful? Give feedback.
-
I updated CESM, CAM and CTSM branches with a new build of git-fleximod. |
Beta Was this translation helpful? Give feedback.
-
I just tried using git-fleximod with the checkout of the latest cesm remove_manage_externals branch. In CESMROOT I'm getting a src directory with the directory structure for cam physics and dynamics. [fischer@izumi CESM_RME]$ ls -R src |
Beta Was this translation helpful? Give feedback.
-
Hi @jedwards4b manage_externals used to return more information when a checkout failed due to a lack of disk space:
fleximod just says it fails but not why:
Would it be possible to pass additional information through to the error handler on the reason for the failure? Thanks! |
Beta Was this translation helpful? Give feedback.
-
Here is a workflow I do routinely, but doesn't work as expected. I will clone a tag, edit my .gitmodules file to reflect what I want and then bin/git-fleximod checkout to get exactly the setup I want. I don't get the updated external, but rather the one that was originally in the clone. Details: [submodule "carma"] bin/gitfleximod checkout (get carma4_05 and not the carma4_01 which I specified and was listed in the git-fleximod output when it ran. |
Beta Was this translation helpful? Give feedback.
-
Repeating the above listed workflow, but substituting the final command with update (instead of checkout): bin/git-fleximod update I do not get anything in the src/physics/carma/base directory (it is empty), and all of the other externals are not populated either. Neither checkout nor update work by themselves to get the requested carma tag. The only way I've been able to get this to work is to do two commands, checkout followed by update. If this is truly the case (and not a bug), then perhaps "checkout" can perform the current "checkout" followed by "update" automatically for the user. |
Beta Was this translation helpful? Give feedback.
-
@jedwards4b - As we get closer to release of this tool, could the documentation perhaps be augmented? Here are some suggestions of what might need to be included:
|
Beta Was this translation helpful? Give feedback.
-
New versions of several component models are available:
|
Beta Was this translation helpful? Give feedback.
-
@jedwards4b - I would propose for CAM - that we have in addition to bin/git-fleximod, a soft link in the bin directory back to your ./lib/git-fleximod/README.md since this directory is hidden from the users. Also, a reminder that README.md needs to be updated with the current implementation (I'm sure it's on your radar). I'm also contemplating having a text file in CAM's home directory |
Beta Was this translation helpful? Give feedback.
-
CESM3_0_beta01 is out and manage_externals has been replaced. |
Beta Was this translation helpful? Give feedback.
-
Background
Requirements
This section should represent a CSEG and community consensus of the requirements that MUST or SHOULD be met by a CESM externals management tool / process. Note that manage_externals currently supports all of these requirements except those explicitly noted below.
Beta Was this translation helpful? Give feedback.
All reactions