| Message ID | 20181012052833.6945-3-luke@diamand.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git-p4: improved unshelving | expand |
Luke Diamand <luke@diamand.org> writes: > The branch detection code looks for branches under refs/remotes/p4/... > and can end up getting confused if there are unshelved changes in > there as well. This happens in the function p4BranchesInGit(). > > Instead, put the unshelved changes into refs/remotes/p4-unshelved/<N>. I am not a p4 user (and not a git-p4 user), so it is a bit hard for me to assess if this is a backward incompatibile change and if so how serious potential breakage to existing users would be. > > -If the target branch in refs/remotes/p4/unshelved already exists, the old one will > +If the target branch in refs/remotes/p4-unshelved already exists, the old one will > be renamed. > > ---- > $ git p4 sync > $ git p4 unshelve 12345 > -$ git show refs/remotes/p4/unshelved/12345 > +$ git show p4/unshelved/12345 Isn't this "p4-unshelved/12345" now?
On Fri, 12 Oct 2018 at 14:45, Junio C Hamano <gitster@pobox.com> wrote: > > Luke Diamand <luke@diamand.org> writes: > > > The branch detection code looks for branches under refs/remotes/p4/... > > and can end up getting confused if there are unshelved changes in > > there as well. This happens in the function p4BranchesInGit(). > > > > Instead, put the unshelved changes into refs/remotes/p4-unshelved/<N>. > > I am not a p4 user (and not a git-p4 user), so it is a bit hard for > me to assess if this is a backward incompatibile change and if so > how serious potential breakage to existing users would be. I don't think it's a particularly serious breakage - it reports the branch it unshelves to, so it should be fairly obvious. However, maybe it would make sense to pull this into a separate commit to make it more obvious? I should have thought of that before submitting. > > > > > -If the target branch in refs/remotes/p4/unshelved already exists, the old one will > > +If the target branch in refs/remotes/p4-unshelved already exists, the old one will > > be renamed. > > > > ---- > > $ git p4 sync > > $ git p4 unshelve 12345 > > -$ git show refs/remotes/p4/unshelved/12345 > > +$ git show p4/unshelved/12345 > > Isn't this "p4-unshelved/12345" now? Yes, I think another reason to pull into a separate commit. Luke >
On Fri, 12 Oct 2018 at 19:19, Luke Diamand <luke@diamand.org> wrote: > > On Fri, 12 Oct 2018 at 14:45, Junio C Hamano <gitster@pobox.com> wrote: > > > > Luke Diamand <luke@diamand.org> writes: > > > > > The branch detection code looks for branches under refs/remotes/p4/... > > > and can end up getting confused if there are unshelved changes in > > > there as well. This happens in the function p4BranchesInGit(). > > > > > > Instead, put the unshelved changes into refs/remotes/p4-unshelved/<N>. > > > > I am not a p4 user (and not a git-p4 user), so it is a bit hard for > > me to assess if this is a backward incompatibile change and if so > > how serious potential breakage to existing users would be. > > I don't think it's a particularly serious breakage - it reports the > branch it unshelves to, so it should be fairly obvious. > > However, maybe it would make sense to pull this into a separate commit > to make it more obvious? I should have thought of that before > submitting. > > > > > > > > > -If the target branch in refs/remotes/p4/unshelved already exists, the old one will > > > +If the target branch in refs/remotes/p4-unshelved already exists, the old one will > > > be renamed. > > > > > > ---- > > > $ git p4 sync > > > $ git p4 unshelve 12345 > > > -$ git show refs/remotes/p4/unshelved/12345 > > > +$ git show p4/unshelved/12345 > > > > Isn't this "p4-unshelved/12345" now? > > Yes, I think another reason to pull into a separate commit. D'oh. It's already in a separate commit. I'll re-roll fixing that documentation. I think it will be fine to change the branch that the unshelving happens into - I think it's very unlikely anyone is basing some automated scripts on this, because of the way that unshelving is used anyway. Luke
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt index 41780a5aa9..c7705ae6e7 100644 --- a/Documentation/git-p4.txt +++ b/Documentation/git-p4.txt @@ -174,7 +174,7 @@ $ git p4 submit --update-shelve 1234 --update-shelve 2345 Unshelve ~~~~~~~~ Unshelving will take a shelved P4 changelist, and produce the equivalent git commit -in the branch refs/remotes/p4/unshelved/<changelist>. +in the branch refs/remotes/p4-unshelved/<changelist>. The git commit is created relative to the current origin revision (HEAD by default). If the shelved changelist's parent revisions differ, git-p4 will refuse to unshelve; @@ -182,13 +182,13 @@ you need to be unshelving onto an equivalent tree. The origin revision can be changed with the "--origin" option. -If the target branch in refs/remotes/p4/unshelved already exists, the old one will +If the target branch in refs/remotes/p4-unshelved already exists, the old one will be renamed. ---- $ git p4 sync $ git p4 unshelve 12345 -$ git show refs/remotes/p4/unshelved/12345 +$ git show p4/unshelved/12345 <submit more changes via p4 to the same files> $ git p4 unshelve 12345 <refuses to unshelve until git is in sync with p4 again> diff --git a/git-p4.py b/git-p4.py index 5701bad06a..76c18a22e9 100755 --- a/git-p4.py +++ b/git-p4.py @@ -3956,7 +3956,8 @@ def __init__(self): ] self.verbose = False self.noCommit = False - self.destbranch = "refs/remotes/p4/unshelved" + self.destbranch = "refs/remotes/p4-unshelved" + self.origin = "p4/master" def renameBranch(self, branch_name): """ Rename the existing branch to branch_name.N diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh index 48ec7679b8..c3d15ceea8 100755 --- a/t/t9832-unshelve.sh +++ b/t/t9832-unshelve.sh @@ -54,8 +54,8 @@ EOF cd "$git" && change=$(last_shelved_change) && git p4 unshelve $change && - git show refs/remotes/p4/unshelved/$change | grep -q "Further description" && - git cherry-pick refs/remotes/p4/unshelved/$change && + git show refs/remotes/p4-unshelved/$change | grep -q "Further description" && + git cherry-pick refs/remotes/p4-unshelved/$change && test_path_is_file file2 && test_cmp file1 "$cli"/file1 && test_cmp file2 "$cli"/file2 && @@ -88,7 +88,7 @@ EOF cd "$git" && change=$(last_shelved_change) && git p4 unshelve $change && - git diff refs/remotes/p4/unshelved/$change.0 refs/remotes/p4/unshelved/$change | grep -q file3 + git diff refs/remotes/p4-unshelved/$change.0 refs/remotes/p4-unshelved/$change | grep -q file3 ) '
The branch detection code looks for branches under refs/remotes/p4/... and can end up getting confused if there are unshelved changes in there as well. This happens in the function p4BranchesInGit(). Instead, put the unshelved changes into refs/remotes/p4-unshelved/<N>. Signed-off-by: Luke Diamand <luke@diamand.org> --- Documentation/git-p4.txt | 6 +++--- git-p4.py | 3 ++- t/t9832-unshelve.sh | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-)