ENH: when installing/getting a submodule, make checkout branch to point to that commit #1370
Conversation
| @@ -327,7 +364,7 @@ def _get_flexible_source_candidates_for_submodule(ds, sm_path, sm_url=None): | |||
| sm_url, | |||
| remote_url if remote_url else ds.path) | |||
|
|
|||
| return clone_urls | |||
| return unique(clone_urls) | |||
There was a problem hiding this comment.
spotted it going through identical URLs -- so fixed with our helper
| @@ -214,7 +215,43 @@ def _install_subds_from_flexible_source(ds, sm_path, sm_url, reckless): | |||
| # do fancy update | |||
| if sm_path in ds.get_subdatasets(absolute=False, recursive=False): | |||
| lgr.debug("Update cloned subdataset {0} in parent".format(subds)) | |||
| # TODO: move all of that into update_submodule ?? | |||
| # TODO: direct mode ramifications? | |||
There was a problem hiding this comment.
started to move into update_submodule but that one is defined in GitRepo, and so far nothing decorated at AnnexRepo level. @bpoldrack -- should it work currently in direct mode somehow?
There was a problem hiding this comment.
I'm too late obviously, but for FTR: Yes, it should. AnnexRepo makes sure, that all the git calls are done with -c core.bare=False automatically in direct mode. It needs to override it only if this isn't sufficient, but in most cases it is.
Codecov Report
@@ Coverage Diff @@
## master #1370 +/- ##
==========================================
- Coverage 89.4% 89.36% -0.05%
==========================================
Files 236 236
Lines 24682 24773 +91
==========================================
+ Hits 22068 22138 +70
- Misses 2614 2635 +21
Continue to review full report at Codecov.
|
|
@bpoldrack @mih -- what do you think? should I implement more safe-guards or should be ok as is for now? |
|
There is indeed the potential to loose something. Especially, if we start to prune things automatically, as suggested in #1371 I think we should have protection against that. |
…as already installed (should not happen actually)
|
ha -- looking back at the code -- I think we are safe since this function is to install (clone) new sub-datasets so there is nothing too loose since we didn't have anything before running it. Nevertheless I have pushed f6f8d69 to
This whole logic/adjustment I guess is also relevant to 'update' action, which should be actually the one worrying about loosing stuff ;) but I did not find other uses of |
|
The reason why you don't find any other use of that function is that
|
|
#1350 is now broken after having merged this PR. The last line of the log snippet below documents the origin of the failure. That line was added in this PR and accesses the I am not sure why this is happening at all. I would appreciate any hints. Thanks. |
|
sorry -- can't find that code within 0.4.1-688-g55689abc of your branch -- 146th line of that file is just "return subds" for me |
|
Yes, i reverted it.
…On Mar 20, 2017 04:21, "Yaroslav Halchenko" ***@***.***> wrote:
sorry -- can't find that code within 0.4.1-688-g55689abc of your branch --
146th line of that file is just "return subds" for me
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#1370 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAIVH8fae1v5ugI16920NV9ejWH40pF8ks5rnfCqgaJpZM4Mc-2s>
.
|
Closes #937
but now I wonder if we should place more safe-guards, e.g. to not perform it if previous position of the branch is not available within any remote, thus signalling that it probably has local changes, thus we shouldn't just 'loose' them.