git fetch remote refs. #80
Conversation
cfelder
force-pushed
the
fetch-ref
branch
2 times, most recently
from
51fecf6
to
0704914
Compare
|
Thanks. I think I sort of understand why you want to do this, but not completely. Please can you give a concrete example of a repository and a corresponding revision which doesn't get fetched during a clone? Also please note I cannot merge this until you fix the test suite. |
cfelder
force-pushed
the
fetch-ref
branch
2 times, most recently
from
81c63a3
to
b9365b2
Compare
|
I fixed the test-issue. Below is an example of a Gerrit Code Review Patch referenced by felder@mbadmin:tmp % git clone https://iffwww.iff.kfa-juelich.de/review/gr
Cloning into 'gr'...
remote: Counting objects: 7852, done
remote: Finding sources: 100% (7852/7852)
remote: Total 7852 (delta 4478), reused 7405 (delta 4478)
Receiving objects: 100% (7852/7852), 26.34 MiB | 8.66 MiB/s, done.
Resolving deltas: 100% (4478/4478), done.
Checking connectivity... done.
felder@mbadmin:tmp % cd gr
felder@mbadmin:gr (master) % git rev-parse --verify refs/changes/61/261/3
fatal: Needed a single revision
felder@mbadmin:gr (master) % git fetch https://iffwww.iff.kfa-juelich.de/review/gr refs/changes/61/261/3
remote: Counting objects: 9, done
remote: Finding sources: 100% (5/5)
remote: Total 5 (delta 4), reused 5 (delta 4)
Unpacking objects: 100% (5/5), done.
From https://iffwww.iff.kfa-juelich.de/review/gr
* branch refs/changes/61/261/3 -> FETCH_HEAD
felder@mbadmin:gr (master) % git update-ref refs/changes/61/261/3 FETCH_HEAD
felder@mbadmin:gr (master) % git rev-parse --verify refs/changes/61/261/3
12a37e46717f1c894c4ae209a7cce86033fdff13 |
| if revision: | ||
| # check if the reference already exists. | ||
| try: | ||
| safe_run(['git', 'rev-parse', '--verify', '--quiet', revision], |
There was a problem hiding this comment.
Please use subprocess.call here. safe_run is only intended to be used when you don't expect the command to fail in normal circumstances.
There was a problem hiding this comment.
I can change it to subprocess.call resulting in some code duplication from safe_run but I just would like to let you know that you are using it in the same way in update_cache_hg. I tried not to duplicate code and not to break the existing style.
Therefore I would like to know if this is really what you want me to do?
There was a problem hiding this comment.
Haha, good catch :-) Actually I didn't write that code. But well done for copying the existing style, even if I don't like it ;-) Yeah I think it would be better not to use safe_run when the command can fail in normal circumstances.
Also I now notice that switch_revision_git currently needs to check if a revision exists. If we're adding a check here, then maybe that's no longer necessary - what do you think?
There was a problem hiding this comment.
The revision check in switch_revision_git is slightly different because it checks against the revs = [x + revision for x in ['origin/', '']]. Since fetch_upstream_git does not call switch_revision_git it would be impossible to change the revision to 'origin/' + revision.
There was a problem hiding this comment.
I'm not sure I follow you. Why would we want to change the revision to 'origin/' + revision? My point was that the following sequence contains redundancy:
git clone- Check if we have the revision we want.
- If not fetch it.
- Check if we have the revision we want.
- Switch to it.
But I suppose it could be argued that step 4 is an extra safeguard against step 3. not doing what we expect. So I'm OK with keeping that, as long as the duplicate code for steps 2. and 4. is refactored into a separate function like I already suggested to do.
|
Thanks that's extremely helpful - I understand perfectly now :) |
| cwd=clone_dir, interactive=sys.stdout.isatty()) | ||
| except SystemExit: | ||
| # fetch reference from url and create locally | ||
| safe_run(['git', 'fetch', url, revision], cwd=clone_dir, |
There was a problem hiding this comment.
If you change this to
['git', 'fetch', url, revision + ':' + revision]
then you can eliminate the update-ref command below.
cfelder
force-pushed
the
fetch-ref
branch
2 times, most recently
from
aa5ac53
to
94d16ba
Compare
|
Thanks so much - really really close to merging now! Just one more request: please can you extract a |
|
ping |
|
Sorry, I didn't notice that you updated - annoyingly, github doesn't send notifications after a push. This now looks fantastic except for one thing - the |
Fetch remote references in order to check them out. switch_revision_git already supports checking out refs if they are known in your cloned repository but when using code review systems e.g. gerrit the references (e.g. refs/changes/61/261/3) will not be cloned from the origin. Therefore fetch the specified reference and create it locally in fetch_upstream_git. Afterwards switch_revision_git can switch to that reference.
In order to run commands which could fail in normal circumstances use run_cmd with raisesysexit=False. This function will not raise a SystemExit exception and returns a tuple of return code and command output which used to evaluate the status of the executed command. safe_cmd calls run_cmd with raisesysexit=True which will raise a SystemExit exception if the return code is non-zero (previous behaviour).
In order to check for existance of git references provide a new method git_ref_exists which will return True if the reference exists in the local repository False otherwise.
|
Done. |
|
Many thanks for your patience and hard work on this! |
|
Thanks for merging my contributions. Are you planning a release v0.6.0? |
|
Yes that makes sense, although we need to fix #82 first. |
Fetch remote references in order to check them out.
switch_revision_git already supports checking out refs if they are known in
your cloned repository but when using code review systems e.g. gerrit the
references will not be cloned from the origin. Therefore fetch the specified
reference and create it locally in fetch_upstream_git. Afterwards
switch_revision_git can switch to that reference.