-
Notifications
You must be signed in to change notification settings - Fork 197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revise documentation on how to use a branch of a component within a CESM checkout #139
Comments
I think there may not be a one-size-fits-all recommendation. I end up doing both approaches depending on my needs. I personally start out with changing the Externals.cfg file and pointing to my branch in it. That is easy and suits my needs a good portion of the time. At other times(listed below), I will remove (or rename) the component in question and by hand do a The times that I retrieve the component by hand are:
I suppose we could document the two approaches along with the times when a user might prefer one over the other. |
Hi all, I really, really prefer to work with the Externals.cfg files. I find that they are an important way to document exactly which versions of which components you are using for your experiments. This is super important for papers and reproducibility. I think it would be very valuable to have manage_externals ignore the changes to an Externals.cfg file, so that all checkouts can be done using one method and one tool. That would be my vote. |
If we were to additionally modify manage_externals to have a flag to allow checkouts to not have detached heads, then I too could depend on it 100% of the time (assuming Kate's suggestion was also addressed). |
@cacraigucar if the top level external points to a branch in a component, by default it checks it out as a detached-head, but you can change it to a branch using git commands, and manage_externals will continue to work correctly after that. Now, it does seem like having an option for manage_externals to checkout the branch properly rather than as a detached head would be useful here. |
The way I recommend is that if the component in question is at the top level (in Externals.cfg) you edit it there and use manage_externals to manage the branch. If you have to point to something that's in a lower level -- right now you have to do it with git commands. One advantage of using manage_externals for Externals.cfg is that the CIME case system documents manage_externals results along the way. So if the top level Externals.cfg file is correct it's going to add it to the documentation of the case or test. It'll also give good documentation when you run the status command. That's part of why I think it's good when you can to use manage_externals for at least the top level. |
@cacraigucar also has a good point about when you have uncommitted mods. Sometimes you have something small that you don't really want to commit or create a branch for. One answer is git's "stash" command that allows you to keep a few of these types of things around with a little less overhead. It still has some overhead though. Personally I usually work around it either by using "git stash" or by saving a local copy of what I changed before running manage_externals and then copy it back in afterwards. Saving a copy works when it's a single file, which is my typical case for wanting to do that. |
@Katetc I'm not sure what you mean by this...
So are you saying it would use the latest version of Externals.cfg in git and ignore the modified version? That would be a problem while you are developing a new branch or tag or even a new git repository for that matter. I guess I could see an option where manage_externals was told to abort if Externals.cfg has been modified. I think maybe you are thinking of the case where you've checked out a given tag, and really want the externals to match exactly what that tag was? If so that does make sense in those cases that I've cloned something that I just want to point to a given very specific tag, and don't want it to ever change from that (like for control simulations). |
@ekluzek , sorry I was a bit vague there. I meant that manage_externals should ignore changes to Externals files in subdirectories. Such as the Externals_CISM.cfg file. Currently, if that one is changed, then manage_externals will not proceed due to the modification. I think it should treat a subdirectory where only an Externals.cfg file is changed as being unchanged. In other words, treat Externals.cfg files in sub directories the same way it does the ones in the main directory. |
Thank you all for your responses so far. For now I just want to reply to @Katetc 's latest comment:
This is a lot easier said than done, for two reasons (and I acknowledge that I'm the one who brought this up based on earlier conversations with @ekluzek , so I'm not pointing a finger at you for asking for it here):
What you're asking for would require parsing the output and ignoring files that match a certain pattern. Note that there is no required naming convention of files like You said:
but it's not that easy. Externals.cfg in the main directory is ignored implicitly, because it isn't a member of any externals.
Sorry, I forgot about point (2) when suggesting this possibility in my original comment. I am now thinking we really shouldn't do this. Now what we could possibly do is generally relax the checking in manage_externals, so that it still allows an update in the case of a dirty sandbox, as long as it isn't trying to update a component that is itself dirty. (So if you have modified But I'm also having trouble reconciling these thoughts with your earlier comment:
For this to work well, don't you need to have your changes committed to a branch, and have that branch pushed to somewhere on GitHub? |
Note, I'm setting up a time to discuss this tomorrow at 11:00am MDT. Ping me if you want to be added to the group. |
Ok, so I see some of the issues here. Thanks for explaining that. There is a lot to consider here and I'm having a hard time framing my thoughts in a coherent message this afternoon. I agree with everything you said Bill, but I find it hard to believe that the ONLY solution to this problem (ie, in my sandbox, I want to run with a different src_cism than the release) is that we need a new wrapper branch every time, or stop using manage_externals (and then your externals don't match the Externals_FOO.cfg file). Though, I wrote another entire comment about how checking your code into a branch is generally a good idea, and maybe we should just suggest branches off people's forks to address this. But, that could be tough for plenty of other reasons. So, I deleted my comment. We can talk more about this at the meeting tomorrow. |
Me too, but @ekluzek I'd like to be included in the discussion tomorrow (hopefully I'll have some clearer thoughts by then) In my mind, it would be great to include the features @Katetc and @cacraigucar are requesting:
So far I've had a couple of awesome idea that were decidedly less awesome by the time I got halfway through writing them up, but maybe others can expand on my half-baked ideas tomorrow :) (Basically, I'd like a |
I just remembered that I already opened this issue: ESMCI/manage_externals#112 . If people feel that's a good idea, then it would take care of this point, at least in some use cases.
We (largely you, @mnlevy1981 , together with @gold2718 ) sketched out a design for this in ESMCI/manage_externals#34. I don't feel this needs more discussion right now. What both of these issues have lacked is someone with the time to implement them. |
Also, my experience with manage_externals is that it is a lot harder than you first imagine to come up with behavior that is robust and doesn't break some other use case. This is not an issue with manage_externals per se, but rather is due to the complexity of the problem it is trying to solve. What has helped before is to have very specific use cases, detailing exactly what you want to be able to do, and what you want the behavior of manage_externals to be. The discussion in this issue has so far been too vague for me to feel like I have a good handle on it. So I'd like to request that, before we have this meeting, people who feel manage_externals should operate differently please write up specific and detailed use cases laying out what they want. I'd like to be able to read these over and think about them a bit before we have a discussion. This probably argues for pushing the meeting back to at least next week. |
I'll comment in ESMCI/manage_externals#112 with my thought of introducing a file that specifies what files are allowed to have changed without triggering an abort. Also, I haven't changed my mind from two years ago (though I'll admit to having forgotten my opinion for many of the intervening months), and still like the workflow specified in ESMCI/manage_externals#34 (comment) |
Sorry for being late in the discussion. Although I have no objection to fully relying on manage_externals, my preference is to recommend that people check out their branch using regular git commands, mainly because many are already familiar and comfortable with git, and I, personally, sometimes find manage_externals an unnecessary level of indirection. Coincidentally, I am in the middle of putting together a “Development and Testing” guideline for MOM6, and working directly with git commands is exactly what I suggest in the document (a work in progress): https://github.com/ESCOMP/MOM_interface/wiki/Development-and-Testing
One enhancement I'd suggest for manage_externals is to have the ability of checking out an external recursively, i.e., together with its git submodules. @gold2718 has recently added the option of getting externals from git submodules, but we still need to list the submodules in an auxiliary Externals file. In the case of MOM6, for instance, we have an interface repository called MOM_interface, which encapsulates the core MOM6 repository. The core MOM6 repository has several submodules that need to be listed in an Externals file in MOM_interface: https://github.com/ESCOMP/MOM_interface/blob/master/Externals_MOM.cfg |
Alper,
We use manage externals with the ufs project where there is a hierarchy of
git submodules and it works there - have you tried it with Mom lately? If
I remember right part of the issue with MOM6 is that we didn't want *all*
the submodules. But I may be misremembering that.
…On Wed, Mar 18, 2020 at 5:43 PM Alper Altuntas ***@***.***> wrote:
Sorry for being late in the discussion. Although I have no objection to
fully relying on manage_externals, my preference is to recommend that
people check out their branch using regular git commands, mainly because
many are already familiar and comfortable with git, and I, personally,
sometimes find manage_externals an unnecessary level of indirection.
Coincidentally, I am in the middle of putting together a “Development and
Testing” guideline for MOM6, and working directly with git commands is
exactly what I suggest in the document (a work in progress):
https://github.com/ESCOMP/MOM_interface/wiki/Development-and-Testing
I'd like to request that, before we have this meeting, people who feel
manage_externals should operate differently please write up specific and
detailed use cases laying out what they want.
One enhancement I'd suggest for manage_externals is to have the ability of
checking out an external recursively, i.e., together with its git
submodules. @gold2718 <https://github.com/gold2718> has recently added
the option of getting externals from git submodules, but we still need to
list the submodules in an auxiliary Externals file. In the case of MOM6,
for instance, we have an interface repository called MOM_interface, which
encapsulates the core MOM6 repository. The core MOM6 repository has several
submodules that need to be listed in an Externals file in MOM_interface:
https://github.com/ESCOMP/MOM_interface/blob/master/Externals_MOM.cfg
If we could instruct manage_externals to check out MOM6 recursively, then
we would not need to maintain a seconday externals file Externals_MOM.cfg
for MOM6 submodules, which would simplify the workflow, especially for
those who prefer to work with git directly.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#139 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABOXUGB27X3HGVUL5S65Y4LRIFMBTANCNFSM4LOX6W5A>
.
--
Jim Edwards
CESM Software Engineer
National Center for Atmospheric Research
Boulder, CO
|
I would be happy to do this. But, yes, I don't think I can get it done today. |
We started the discussion on this. Half of the half-dozen people on the call would do git commands for branches, and the other half would make branches for the purpose of having manage_externals take care of everything. The other use-case that makes a difference is if it's the production environment for running simulations versus development environment. We all agreed that in the production environment you want manage_externals to do everything for you, and you want it to make sure everything is correct and not allow something incorrect to exist unchallenged. So the current behavior is probably the best. But, that also sets up a framework where our development environment is inconsistent with the production environment. Note, we have a similar but worse problem with people who develop using SourceMods -- there are always issues when we move them into the git world. This difference also means that sometimes you forget to update and commit the Externals files for instance. So I think the primary thing we want to do is to add some additional features as options to manage_externals to make it more useful in the development framework, and so it would allow developers to use it exclusively for external management. |
Note, also that @jedwards4b pointed out a feature in manage_externals to us that most of us hadn't realized. A positional argument to manage_externals is to checkout the list of externals that you give. So for example if you just want to update cism you would do...
You could also just update a single externals file using the "-e" (--externals option). Like this...
|
7b6d92e Merge pull request ESCOMP#198 from johnpaulalex/gitdir 927ce3a Merge pull request ESCOMP#197 from johnpaulalex/testpath a04f114 Merge pull request ESCOMP#196 from johnpaulalex/readmod d9c14bf Change the rest of the methods to use -C. Still some usage of getcwd in test_unit_repository_git. 332b106 Fix incorrect logged path of checkout_externals in test_sys_checkout: it was basically the parent of the current directory, which varies throughout the test. (it called abspath with '{0}/../../', which adds arbitrary and not-interpolated subdir '{0}' to the path, then removes it and removes one more level). 932a749 Remove printlog from read_gitmodules_file since read_externals_description_file() already has a nearly-the-same printlog (but add it to the other caller). 5d13719 Merge pull request ESCOMP#195 from johnpaulalex/check_repo 4233954 Update utest to mock _git_remote_verbose in a new way, since it is now called via the GitRepository class rather than on the specific GitRepository instance. d7a42ae Check that desired repo was actually checked out. 71596bb Merge pull request ESCOMP#194 from johnpaulalex/manic2 4c96e82 Make the MANIC_TEST_BARE_REPO_ROOT env var special - give it a constant for easy tracking, and automatically tear it down after each test. 259bfc0 test_sys_checkout: use actual paths in on-the-fly configs rather than MANIC_TEST_BARE_REPO_ROOT env var. This will make it easier to test (in the near future) that checkout_externals actually checked out the desired repo dir. 557bbd6 Merge pull request ESCOMP#193 from johnpaulalex/manic 5314eed Remove MANIC_TEST_TMP_REPO_ROOT environment variable in favor of module-level variable. 345fc1e Merge pull request ESCOMP#191 from johnpaulalex/test_doc12 2117b84 test_sys_checkout: verify that basic by-tag/branch/hash tests actually take us to the correct git tag/branch/hash. 94d6e5f Merge pull request ESCOMP#190 from johnpaulalex/test_doc11 3ff33a6 Inline local-path-creation methods 47dea7f Merge pull request ESCOMP#189 from johnpaulalex/test_doc10 9ea75cb Grab-bag of renamings: Remove redundant _NAME from repo constants, and consistently add _REPO suffix (This causes the majority of diffs). c0c847e Merge pull request ESCOMP#188 from johnpaulalex/test_doc9 2dd5ce0 test_sys_checkout.py: only check for correct 'required' or 'optional' state in the test that exercises required vs optional behavior. Removed a lot of boilerplate. eb30859 Merge pull request ESCOMP#187 from johnpaulalex/test_doc8 1832e1f test_sys_checkout: Simplify many tests to only use a single external. 8689d61 Merge pull request ESCOMP#186 from johnpaulalex/test_doc7 fbee425 Grab bag of test_sys_checkout cleanups: Doc inside of each test more clearly/consistently. TestSysCheckoutSVN didn’t get the inlining-of-helper-methods treatment, now it has that. Move various standalone repo helper methods (like create_branch) into a RepoUtils class. README.md was missing newlines when rendered as markdown. Doc the return value of checkout.main Fix test_container_exclude_component - it was looking for the wrong key (which is never present); now it looks for the correct key. f0ed44a Merge pull request ESCOMP#185 from johnpaulalex/test_doc6 a3d59f5 Merge pull request ESCOMP#184 from johnpaulalex/test_doc5 5329c8b test_sys_checkout: Inline config generation functions that are only called once. 464f2c7 test_sys_checkout: Inline another layer (per-config-file checks). Rename the 4 methods that are used multiple times, to reflect what they do rather than what they're called. 8872c0d Merge pull request ESCOMP#183 from johnpaulalex/doc_test4 c045335 Merge pull request ESCOMP#182 from johnpaulalex/doc_test3 c583b95 Merge pull request ESCOMP#181 from johnpaulalex/doc_test2 e01cfe2 test_sys_checkout: less confusing handling of return values from checkout_externals. Specifically, when doing a checkout, don't return tree_status from _before_ the checkout. Make a new wrapper to call checkout_externals a second time, to calculate the new status after a checkout (very frequent pattern). 2328681 test_sys_checkout: Remove another layer (which generates test component names) c3717b6 Merge pull request ESCOMP#180 from johnpaulalex/doc_test 36d7a44 test_sys_checkout.py: remove one layer of functions (that check for local status enums). No-op. 2c4584b More documentation about tests: * contents of test repositories (n a new README.md) * various constants in test_sys_checkout.py that point to those contents, and terminology like container/simple/mixed. * in each test method, the scenarios being tested. * The coupling between test methods. 55e74bd Merge pull request ESCOMP#179 from johnpaulalex/circ 66be842 Remove circular dependency by making _External stop doing tricky things with sourcetrees. 82d3b24 Merge pull request ESCOMP#178 from johnpaulalex/test_doc 3223f49 Additional documentation of system tests - global variables, method descriptions. 45b7c01 Merge pull request ESCOMP#177 from jedwards4b/git_workflow ace90b2 try setting credentials this way f4d6aa9 try setting credentials this way 1d61a69 use this to set git credentials 7f9d330 use this to set git credentials 5ac731b add tmate code 836847b get git workflow working dcd462d Merge pull request ESCOMP#176 from jedwards4b/add_github_testing 2d2479e Merge pull request ESCOMP#175 from johnpaulalex/fix 711a53f add github testing of prs and automatic tagging of main cfe0f88 fix typos 5665d61 Fix broken checkout behavior introduced by PR ESCOMP#172. 27909e2 Merge pull request ESCOMP#173 from johnpaulalex/readall 00ad044 Further tiny refactorings and docs of checkout API (no-op). Remove unused load_all param in _External.checkout(). Rename _External.checkout_externals() to checkout_subexternals(), to remove the ambiguity about whether the main external pointed to by the _External is itelf checked out (it is not) Clarify load_all documentation - it’s always recursive, but applies different criteria at each level. Rename variables in checkout.py (e.g. ext_description) to match the equivalent code in sourcetree.py. 2ea3d1a Merge pull request ESCOMP#172 from johnpaulalex/fixit 43bf809 Merge pull request ESCOMP#171 from johnpaulalex/docstatus e6aa7d2 Merge pull request ESCOMP#170 from johnpaulalex/printdir adbd715 On checkout, refresh locally installed optional packages regardless of whether -o is passed in. add0745 Comment tweaks, and fix 'ppath' typo 696527c Document the format of various status dictionaries, and the various paths and path components within an _External. c677b94 When processing an external, print out its path in addition to the base filename (to disambiguate all the externals.cfg's) 975d7fd Merge pull request ESCOMP#169 from johnpaulalex/docfix_branch 09709e3 Document _Externals.status(). The original comment was apparently copy-pasted from checkout(). 1d880e0 Merge pull request ESCOMP#167 from billsacks/fix_svn_on_windows 3510da8 Tweak a unit test to improve coverage eb7fc13 Handle the possibility that the URL already ends with '/' 02ea87e Fix svn URLs on Windows b1c02ab Merge pull request ESCOMP#165 from gold2718/doc_fix 9f4be8c Add documentation about externals = None feature a3b3a03 Merge pull request ESCOMP#162 from ESMCI/fischer/python3 d4f1b1e Change shebang lines to python3 2fd941a Merge pull request ESCOMP#158 from billsacks/modified_solution de08dc2 Add another option for when an external is in a modified state e954582 Merge pull request ESCOMP#156 from billsacks/onbranch_show_hash 952e44d Change output: put tag/hash before branch name 1028843 Fix pre-existing pylint issues 01b13f7 When on a branch, show tag/hash, too 39ad532 Merge pull request ESCOMP#150 from gold2718/fix_combo_config 75f8f02 Merge pull request ESCOMP#152 from jedwards4b/sort_by_local_path 42687bd remove commented code 29e26af fix pylint issues 7c9f3c6 add a test for nested repo checkout 75c5353 fix spacing 24a3726 improve sorting, checkout externals with each comp 29f45b0 remove py2 test and fix super call 880a4e7 remove decode 1c53be8 no need for set call 36c56db simplier fix for issue dc67cc6 simpler solution b32c6fc fix to allow submodule name different from path 5b5e1c2 Merge pull request ESCOMP#144 from billsacks/improve_errmsg c983863 Add another option for dealing with modified externals 59ce252 Add some details to the error message when externals are modified be5a1a4 Merge pull request ESCOMP#143 from jedwards4b/add_exclude 2aa014a fix lint issue 49cd5e8 fix lint issues 418173f Added tests for ExternalsDescriptionDict afab352 fix lint issue be85b7d fix the test a580a57 push test d437108 add a test 21affe3 fix formatting issue 72e6b64 add an exclude option c33a3bd Merge pull request ESCOMP#139 from jedwards4b/ignore_branch_prop b124a9a ignore this silently, we use a hash so it does not matter git-subtree-dir: manage_externals git-subtree-split: 7b6d92e
7b6d92e Merge pull request #198 from johnpaulalex/gitdir 927ce3a Merge pull request #197 from johnpaulalex/testpath a04f114 Merge pull request #196 from johnpaulalex/readmod d9c14bf Change the rest of the methods to use -C. Still some usage of getcwd in test_unit_repository_git. 332b106 Fix incorrect logged path of checkout_externals in test_sys_checkout: it was basically the parent of the current directory, which varies throughout the test. (it called abspath with '{0}/../../', which adds arbitrary and not-interpolated subdir '{0}' to the path, then removes it and removes one more level). 932a749 Remove printlog from read_gitmodules_file since read_externals_description_file() already has a nearly-the-same printlog (but add it to the other caller). 5d13719 Merge pull request #195 from johnpaulalex/check_repo 4233954 Update utest to mock _git_remote_verbose in a new way, since it is now called via the GitRepository class rather than on the specific GitRepository instance. d7a42ae Check that desired repo was actually checked out. 71596bb Merge pull request #194 from johnpaulalex/manic2 4c96e82 Make the MANIC_TEST_BARE_REPO_ROOT env var special - give it a constant for easy tracking, and automatically tear it down after each test. 259bfc0 test_sys_checkout: use actual paths in on-the-fly configs rather than MANIC_TEST_BARE_REPO_ROOT env var. This will make it easier to test (in the near future) that checkout_externals actually checked out the desired repo dir. 557bbd6 Merge pull request #193 from johnpaulalex/manic 5314eed Remove MANIC_TEST_TMP_REPO_ROOT environment variable in favor of module-level variable. 345fc1e Merge pull request #191 from johnpaulalex/test_doc12 2117b84 test_sys_checkout: verify that basic by-tag/branch/hash tests actually take us to the correct git tag/branch/hash. 94d6e5f Merge pull request #190 from johnpaulalex/test_doc11 3ff33a6 Inline local-path-creation methods 47dea7f Merge pull request #189 from johnpaulalex/test_doc10 9ea75cb Grab-bag of renamings: Remove redundant _NAME from repo constants, and consistently add _REPO suffix (This causes the majority of diffs). c0c847e Merge pull request #188 from johnpaulalex/test_doc9 2dd5ce0 test_sys_checkout.py: only check for correct 'required' or 'optional' state in the test that exercises required vs optional behavior. Removed a lot of boilerplate. eb30859 Merge pull request #187 from johnpaulalex/test_doc8 1832e1f test_sys_checkout: Simplify many tests to only use a single external. 8689d61 Merge pull request #186 from johnpaulalex/test_doc7 fbee425 Grab bag of test_sys_checkout cleanups: Doc inside of each test more clearly/consistently. TestSysCheckoutSVN didn’t get the inlining-of-helper-methods treatment, now it has that. Move various standalone repo helper methods (like create_branch) into a RepoUtils class. README.md was missing newlines when rendered as markdown. Doc the return value of checkout.main Fix test_container_exclude_component - it was looking for the wrong key (which is never present); now it looks for the correct key. f0ed44a Merge pull request #185 from johnpaulalex/test_doc6 a3d59f5 Merge pull request #184 from johnpaulalex/test_doc5 5329c8b test_sys_checkout: Inline config generation functions that are only called once. 464f2c7 test_sys_checkout: Inline another layer (per-config-file checks). Rename the 4 methods that are used multiple times, to reflect what they do rather than what they're called. 8872c0d Merge pull request #183 from johnpaulalex/doc_test4 c045335 Merge pull request #182 from johnpaulalex/doc_test3 c583b95 Merge pull request #181 from johnpaulalex/doc_test2 e01cfe2 test_sys_checkout: less confusing handling of return values from checkout_externals. Specifically, when doing a checkout, don't return tree_status from _before_ the checkout. Make a new wrapper to call checkout_externals a second time, to calculate the new status after a checkout (very frequent pattern). 2328681 test_sys_checkout: Remove another layer (which generates test component names) c3717b6 Merge pull request #180 from johnpaulalex/doc_test 36d7a44 test_sys_checkout.py: remove one layer of functions (that check for local status enums). No-op. 2c4584b More documentation about tests: * contents of test repositories (n a new README.md) * various constants in test_sys_checkout.py that point to those contents, and terminology like container/simple/mixed. * in each test method, the scenarios being tested. * The coupling between test methods. 55e74bd Merge pull request #179 from johnpaulalex/circ 66be842 Remove circular dependency by making _External stop doing tricky things with sourcetrees. 82d3b24 Merge pull request #178 from johnpaulalex/test_doc 3223f49 Additional documentation of system tests - global variables, method descriptions. 45b7c01 Merge pull request #177 from jedwards4b/git_workflow ace90b2 try setting credentials this way f4d6aa9 try setting credentials this way 1d61a69 use this to set git credentials 7f9d330 use this to set git credentials 5ac731b add tmate code 836847b get git workflow working dcd462d Merge pull request #176 from jedwards4b/add_github_testing 2d2479e Merge pull request #175 from johnpaulalex/fix 711a53f add github testing of prs and automatic tagging of main cfe0f88 fix typos 5665d61 Fix broken checkout behavior introduced by PR #172. 27909e2 Merge pull request #173 from johnpaulalex/readall 00ad044 Further tiny refactorings and docs of checkout API (no-op). Remove unused load_all param in _External.checkout(). Rename _External.checkout_externals() to checkout_subexternals(), to remove the ambiguity about whether the main external pointed to by the _External is itelf checked out (it is not) Clarify load_all documentation - it’s always recursive, but applies different criteria at each level. Rename variables in checkout.py (e.g. ext_description) to match the equivalent code in sourcetree.py. 2ea3d1a Merge pull request #172 from johnpaulalex/fixit 43bf809 Merge pull request #171 from johnpaulalex/docstatus e6aa7d2 Merge pull request #170 from johnpaulalex/printdir adbd715 On checkout, refresh locally installed optional packages regardless of whether -o is passed in. add0745 Comment tweaks, and fix 'ppath' typo 696527c Document the format of various status dictionaries, and the various paths and path components within an _External. c677b94 When processing an external, print out its path in addition to the base filename (to disambiguate all the externals.cfg's) 975d7fd Merge pull request #169 from johnpaulalex/docfix_branch 09709e3 Document _Externals.status(). The original comment was apparently copy-pasted from checkout(). 1d880e0 Merge pull request #167 from billsacks/fix_svn_on_windows 3510da8 Tweak a unit test to improve coverage eb7fc13 Handle the possibility that the URL already ends with '/' 02ea87e Fix svn URLs on Windows b1c02ab Merge pull request #165 from gold2718/doc_fix 9f4be8c Add documentation about externals = None feature a3b3a03 Merge pull request #162 from ESMCI/fischer/python3 d4f1b1e Change shebang lines to python3 2fd941a Merge pull request #158 from billsacks/modified_solution de08dc2 Add another option for when an external is in a modified state e954582 Merge pull request #156 from billsacks/onbranch_show_hash 952e44d Change output: put tag/hash before branch name 1028843 Fix pre-existing pylint issues 01b13f7 When on a branch, show tag/hash, too 39ad532 Merge pull request #150 from gold2718/fix_combo_config 75f8f02 Merge pull request #152 from jedwards4b/sort_by_local_path 42687bd remove commented code 29e26af fix pylint issues 7c9f3c6 add a test for nested repo checkout 75c5353 fix spacing 24a3726 improve sorting, checkout externals with each comp 29f45b0 remove py2 test and fix super call 880a4e7 remove decode 1c53be8 no need for set call 36c56db simplier fix for issue dc67cc6 simpler solution b32c6fc fix to allow submodule name different from path 5b5e1c2 Merge pull request #144 from billsacks/improve_errmsg c983863 Add another option for dealing with modified externals 59ce252 Add some details to the error message when externals are modified be5a1a4 Merge pull request #143 from jedwards4b/add_exclude 2aa014a fix lint issue 49cd5e8 fix lint issues 418173f Added tests for ExternalsDescriptionDict afab352 fix lint issue be85b7d fix the test a580a57 push test d437108 add a test 21affe3 fix formatting issue 72e6b64 add an exclude option c33a3bd Merge pull request #139 from jedwards4b/ignore_branch_prop b124a9a ignore this silently, we use a hash so it does not matter git-subtree-dir: manage_externals git-subtree-split: 84d65c37f7288576f3a5cd7e4d4fd9f12ac0dece
876b344 Merge pull request #208 from jedwards4b/add_version_file 46b2eb9 improve workflow name 1506bbf Need to update file before creating version cb06623 use github workflow to update version.txt 3d13177 update version.txt ba00e50 Merge branch 'main' into add_version_file 757fed1 update version.txt cd64e76 add version.txt and pre-push hook 0f884bf Merge pull request #205 from jedwards4b/sunset_svn_git_access 82a5edf merge in billsacks:svn_testing_no_github 17532c1 Use a local svn repo for testing 9c90434 different method to determine if in tests 539952e remove debug print statement cc5434f fix submodule testing 1d7f288 remove broken tests 04e94a5 provide a meaningful error message 38bcc0a Merge pull request #201 from jedwards4b/partial_match b4466a5 remove debug print statement c3cf3ec fix issue with partial branch match 7b6d92e Merge pull request #198 from johnpaulalex/gitdir 927ce3a Merge pull request #197 from johnpaulalex/testpath a04f114 Merge pull request #196 from johnpaulalex/readmod d9c14bf Change the rest of the methods to use -C. Still some usage of getcwd in test_unit_repository_git. 332b106 Fix incorrect logged path of checkout_externals in test_sys_checkout: it was basically the parent of the current directory, which varies throughout the test. (it called abspath with '{0}/../../', which adds arbitrary and not-interpolated subdir '{0}' to the path, then removes it and removes one more level). 932a749 Remove printlog from read_gitmodules_file since read_externals_description_file() already has a nearly-the-same printlog (but add it to the other caller). 5d13719 Merge pull request #195 from johnpaulalex/check_repo 4233954 Update utest to mock _git_remote_verbose in a new way, since it is now called via the GitRepository class rather than on the specific GitRepository instance. d7a42ae Check that desired repo was actually checked out. 71596bb Merge pull request #194 from johnpaulalex/manic2 4c96e82 Make the MANIC_TEST_BARE_REPO_ROOT env var special - give it a constant for easy tracking, and automatically tear it down after each test. 259bfc0 test_sys_checkout: use actual paths in on-the-fly configs rather than MANIC_TEST_BARE_REPO_ROOT env var. This will make it easier to test (in the near future) that checkout_externals actually checked out the desired repo dir. 557bbd6 Merge pull request #193 from johnpaulalex/manic 5314eed Remove MANIC_TEST_TMP_REPO_ROOT environment variable in favor of module-level variable. 345fc1e Merge pull request #191 from johnpaulalex/test_doc12 2117b84 test_sys_checkout: verify that basic by-tag/branch/hash tests actually take us to the correct git tag/branch/hash. 94d6e5f Merge pull request #190 from johnpaulalex/test_doc11 3ff33a6 Inline local-path-creation methods 47dea7f Merge pull request #189 from johnpaulalex/test_doc10 9ea75cb Grab-bag of renamings: Remove redundant _NAME from repo constants, and consistently add _REPO suffix (This causes the majority of diffs). c0c847e Merge pull request #188 from johnpaulalex/test_doc9 2dd5ce0 test_sys_checkout.py: only check for correct 'required' or 'optional' state in the test that exercises required vs optional behavior. Removed a lot of boilerplate. eb30859 Merge pull request #187 from johnpaulalex/test_doc8 1832e1f test_sys_checkout: Simplify many tests to only use a single external. 8689d61 Merge pull request #186 from johnpaulalex/test_doc7 fbee425 Grab bag of test_sys_checkout cleanups: Doc inside of each test more clearly/consistently. TestSysCheckoutSVN didn’t get the inlining-of-helper-methods treatment, now it has that. Move various standalone repo helper methods (like create_branch) into a RepoUtils class. README.md was missing newlines when rendered as markdown. Doc the return value of checkout.main Fix test_container_exclude_component - it was looking for the wrong key (which is never present); now it looks for the correct key. f0ed44a Merge pull request #185 from johnpaulalex/test_doc6 a3d59f5 Merge pull request #184 from johnpaulalex/test_doc5 5329c8b test_sys_checkout: Inline config generation functions that are only called once. 464f2c7 test_sys_checkout: Inline another layer (per-config-file checks). Rename the 4 methods that are used multiple times, to reflect what they do rather than what they're called. 8872c0d Merge pull request #183 from johnpaulalex/doc_test4 c045335 Merge pull request #182 from johnpaulalex/doc_test3 c583b95 Merge pull request #181 from johnpaulalex/doc_test2 e01cfe2 test_sys_checkout: less confusing handling of return values from checkout_externals. Specifically, when doing a checkout, don't return tree_status from _before_ the checkout. Make a new wrapper to call checkout_externals a second time, to calculate the new status after a checkout (very frequent pattern). 2328681 test_sys_checkout: Remove another layer (which generates test component names) c3717b6 Merge pull request #180 from johnpaulalex/doc_test 36d7a44 test_sys_checkout.py: remove one layer of functions (that check for local status enums). No-op. 2c4584b More documentation about tests: * contents of test repositories (n a new README.md) * various constants in test_sys_checkout.py that point to those contents, and terminology like container/simple/mixed. * in each test method, the scenarios being tested. * The coupling between test methods. 55e74bd Merge pull request #179 from johnpaulalex/circ 66be842 Remove circular dependency by making _External stop doing tricky things with sourcetrees. 82d3b24 Merge pull request #178 from johnpaulalex/test_doc 3223f49 Additional documentation of system tests - global variables, method descriptions. 45b7c01 Merge pull request #177 from jedwards4b/git_workflow ace90b2 try setting credentials this way f4d6aa9 try setting credentials this way 1d61a69 use this to set git credentials 7f9d330 use this to set git credentials 5ac731b add tmate code 836847b get git workflow working dcd462d Merge pull request #176 from jedwards4b/add_github_testing 2d2479e Merge pull request #175 from johnpaulalex/fix 711a53f add github testing of prs and automatic tagging of main cfe0f88 fix typos 5665d61 Fix broken checkout behavior introduced by PR #172. 27909e2 Merge pull request #173 from johnpaulalex/readall 00ad044 Further tiny refactorings and docs of checkout API (no-op). Remove unused load_all param in _External.checkout(). Rename _External.checkout_externals() to checkout_subexternals(), to remove the ambiguity about whether the main external pointed to by the _External is itelf checked out (it is not) Clarify load_all documentation - it’s always recursive, but applies different criteria at each level. Rename variables in checkout.py (e.g. ext_description) to match the equivalent code in sourcetree.py. 2ea3d1a Merge pull request #172 from johnpaulalex/fixit 43bf809 Merge pull request #171 from johnpaulalex/docstatus e6aa7d2 Merge pull request #170 from johnpaulalex/printdir adbd715 On checkout, refresh locally installed optional packages regardless of whether -o is passed in. add0745 Comment tweaks, and fix 'ppath' typo 696527c Document the format of various status dictionaries, and the various paths and path components within an _External. c677b94 When processing an external, print out its path in addition to the base filename (to disambiguate all the externals.cfg's) 975d7fd Merge pull request #169 from johnpaulalex/docfix_branch 09709e3 Document _Externals.status(). The original comment was apparently copy-pasted from checkout(). 1d880e0 Merge pull request #167 from billsacks/fix_svn_on_windows 3510da8 Tweak a unit test to improve coverage eb7fc13 Handle the possibility that the URL already ends with '/' 02ea87e Fix svn URLs on Windows b1c02ab Merge pull request #165 from gold2718/doc_fix 9f4be8c Add documentation about externals = None feature a3b3a03 Merge pull request #162 from ESMCI/fischer/python3 d4f1b1e Change shebang lines to python3 2fd941a Merge pull request #158 from billsacks/modified_solution de08dc2 Add another option for when an external is in a modified state e954582 Merge pull request #156 from billsacks/onbranch_show_hash 952e44d Change output: put tag/hash before branch name 1028843 Fix pre-existing pylint issues 01b13f7 When on a branch, show tag/hash, too 39ad532 Merge pull request #150 from gold2718/fix_combo_config 75f8f02 Merge pull request #152 from jedwards4b/sort_by_local_path 42687bd remove commented code 29e26af fix pylint issues 7c9f3c6 add a test for nested repo checkout 75c5353 fix spacing 24a3726 improve sorting, checkout externals with each comp 29f45b0 remove py2 test and fix super call 880a4e7 remove decode 1c53be8 no need for set call 36c56db simplier fix for issue dc67cc6 simpler solution b32c6fc fix to allow submodule name different from path 5b5e1c2 Merge pull request #144 from billsacks/improve_errmsg c983863 Add another option for dealing with modified externals 59ce252 Add some details to the error message when externals are modified be5a1a4 Merge pull request #143 from jedwards4b/add_exclude 2aa014a fix lint issue 49cd5e8 fix lint issues 418173f Added tests for ExternalsDescriptionDict afab352 fix lint issue be85b7d fix the test a580a57 push test d437108 add a test 21affe3 fix formatting issue 72e6b64 add an exclude option c33a3bd Merge pull request #139 from jedwards4b/ignore_branch_prop b124a9a ignore this silently, we use a hash so it does not matter git-subtree-dir: manage_externals git-subtree-split: 876b3440e7356fed2bf417659a5f7b5527a92a2f
I periodically get questions about how to point to a branch of a component (say, CAM, CTSM or CISM) within a CESM checkout (most recently from @Ivanderkelen). We have some documentation in the CESM README file, but I'm not sure that what we have is actually the method that makes the most sense. In particular, there are at least two problems with telling people to point to their branch in Externals.cfg:
Support for branches with manage_externals still leaves a lot to be desired (see Handle checking out a branch when that branch is already checked out ESMCI/manage_externals#34)
Additional problems are caused if you change an Externals.cfg file in the middle of the tree (rather than the top-level file). For example, if you want to point to a different version of FATES (an external of CTSM), you might be tempted to modify the file
components/clm/Externals_CLM.cfg
. However, if you do that, when you rerun manage_externals from the top level (from the root of the CESM clone), you will get an error because thecomponents/clm
external has a modified file. (@ekluzek has suggested allowing checkout_externals to proceed if there are modifications in these Externals files. This may be worth considering, although it might be hard to implement and the implications should be thought through carefully.)So I'm inclined to recommend that people checkout their branch using regular git commands. For example, if you want to checkout your branch of CTSM in a CESM checkout, I would recommend getting CESM as normal and doing an initial run of
manage_externals/checkout_externals
. Then do:However, this isn't completely straightforward if your branch is of a component that has its own sub-externals (e.g., you have a branch of CTSM, which has a FATES external). In this case, I would probably still recommend using the above procedure to get your branch, but then getting any sub-externals by running the following from within
components/clm
(NOT from the CESM top level):I'd like to hear some thoughts from others. I don't feel in a rush to update this documentation, so I think it's best if we take some time to gather thoughts until we feel pretty happy with a recommendation for users.
@gold2718 @cacraigucar @nusbaume @ekluzek @Katetc @mnlevy1981 @alperaltuntas @jedwards4b @mvertens @rsdunlapiv @uturuncoglu
The text was updated successfully, but these errors were encountered: