Skip to content
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

Open
billsacks opened this issue Mar 18, 2020 · 20 comments

Comments

@billsacks
Copy link
Member

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:

  1. 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)

  2. 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 the components/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:

cd components/clm
git remote add ...
git fetch ...
git checkout ...

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):

./manage_externals/checkout_externals clm

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

@cacraigucar
Copy link
Contributor

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
git clone ...
git checkout...

The times that I retrieve the component by hand are:

  • I have made mods anywhere and checkout_externals doesn't work
  • I plan on making mods to my fork, so I don't have a detached head and can commit my changes easily

I suppose we could document the two approaches along with the times when a user might prefer one over the other.

@Katetc
Copy link
Contributor

Katetc commented Mar 18, 2020

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.

@cacraigucar
Copy link
Contributor

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).

@ekluzek
Copy link
Contributor

ekluzek commented Mar 18, 2020

@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.

@ekluzek
Copy link
Contributor

ekluzek commented Mar 18, 2020

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.

@ekluzek
Copy link
Contributor

ekluzek commented Mar 18, 2020

@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.

@ekluzek
Copy link
Contributor

ekluzek commented Mar 18, 2020

@Katetc I'm not sure what you mean by this...

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.

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).

@Katetc
Copy link
Contributor

Katetc commented Mar 18, 2020

@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.

@billsacks
Copy link
Member Author

Thank you all for your responses so far. For now I just want to reply to @Katetc 's latest comment:

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.

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):

  1. Currently, manage_externals just checks the git status of each external; if there are modified files, it calls the tree dirty:

https://github.com/ESMCI/manage_externals/blob/c33a3bd2a856dec33febf7f3fda30a4b0b9af608/manic/repository_git.py#L572-L597

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 Externals_CLM.cfg - that's just a convention. So this gets even trickier to do robustly.

You said:

In other words, treat Externals.cfg files in sub directories the same way it does the ones in the main directory.

but it's not that easy. Externals.cfg in the main directory is ignored implicitly, because it isn't a member of any externals.

  1. Assuming we could take care of (1), then there is the issue of what happens if if manage_externals tries to update an external that has a modified Externals_FOO.xml file. In some cases, this would lead to problems in the update, which is what we were trying hard to avoid. For this reason, I feel that ignoring some unmodified files is ill-advised.

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 components/cism/Externals_CISM.cfg, then manage_externals will run happily as long as the top-level Externals.cfg isn't trying to change components/cism.)

But I'm also having trouble reconciling these thoughts with your earlier comment:

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.

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?

@ekluzek
Copy link
Contributor

ekluzek commented Mar 18, 2020

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.

@Katetc
Copy link
Contributor

Katetc commented Mar 18, 2020

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.

@mnlevy1981
Copy link
Contributor

There is a lot to consider here and I'm having a hard time framing my thoughts in a coherent message this afternoon

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:

  1. A way to signal that to manage_externals that some files can differ from what's currently in git without aborting the entire checkout
  2. A way to specify that you want to end up on the head of the branch instead of in a detached head state

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 .manage_externals_ignore file that signals to manage_externals that it's okay if a specific file has been modified, but I think it maybe only works in the very narrow scope where the current checkout / detached head matches where manage_externals wants it to be? That avoids the pitfall of pattern-matching @billsacks mentioned in #139 (comment), while also matching his second criteria for when to allow "dirty" checkouts... but with the added safety feature that only certain files are allowed to be modified)

@billsacks
Copy link
Member Author

  1. A way to signal that to manage_externals that some files can differ from what's currently in git without aborting the entire checkout

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.

A way to specify that you want to end up on the head of the branch instead of in a detached head state

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.

@billsacks
Copy link
Member Author

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.

@mnlevy1981
Copy link
Contributor

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)

@alperaltuntas
Copy link
Member

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 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.

@jedwards4b
Copy link
Contributor

jedwards4b commented Mar 18, 2020 via email

@Katetc
Copy link
Contributor

Katetc commented Mar 19, 2020

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 would be happy to do this. But, yes, I don't think I can get it done today.

@ekluzek
Copy link
Contributor

ekluzek commented Mar 19, 2020

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.

@ekluzek
Copy link
Contributor

ekluzek commented Mar 19, 2020

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...

[eureka:~/Sandboxes/ctsm_relfatesndepmiscupdate] erik% ./manage_externals/checkout_externals cism
./manage_externals/checkout_externals cism
Processing externals description file : Externals.cfg
Processing externals description file : Externals_CISM.cfg
Checking status of externals: cism, source_cism,
Checking out externals: cism,
Processing externals description file : Externals_CISM.cfg
Checking out externals: source_cism,

You could also just update a single externals file using the "-e" (--externals option). Like this...
(Make sure you are in the directory where that externals file exists)

[eureka:~/Sandboxes/ctsm_relfatesndepmiscupdate] erik% ./manage_externals/checkout_externals -e Externals_CLM.cfg
./manage_externals/checkout_externals -e Externals_CLM.cfg
Processing externals description file : Externals_CLM.cfg
Checking status of externals: fates, ptclm,
Checking out externals: fates, ptclm,

billsacks added a commit to billsacks/cesm that referenced this issue Mar 3, 2023
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
jedwards4b added a commit that referenced this issue Apr 20, 2023
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
fischer-ncar added a commit that referenced this issue Dec 19, 2023
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs Prioritization
Development

No branches or pull requests

7 participants