threads / discuss / 32521

Suggested improvements to the git-p4 documentation (branch-related)

Subject: Suggested improvements to the git-p4 documentation (branch-related)

## tl;dr

6 messages between Jan 3, 2013 and Feb 23, 2013.

replies: 5people: 3as markdown or json

Olivier Delalleau· Jan 3, 2013, 20:58 UTC · lore
Hi,

While struggling to get git-p4 to work properly with branches, I thought the documentation on http://git-scm.com/docs/git-p4 could use some improvements:

1. At the end of the "Branch detection" section, the following
commands are provided (for when you want to explicitly provide branch
mappings to git-p4):

git config git-p4.branchList main:branch1 git p4 clone --detect-branches //depot@all

The second command should end with a dot (".") because the first command only works if you are already in a git-initialized folder. Thus I would also suggest to add "git init" as first command to type.

2. Even though having a "main" branch is standard in Perforce, it
would be worth mentioning what happens when you don't: there is a
message "Could not detect main branch. No checkout/master branch
created" output by the "git p4 clone" command. However, it will still
work if you manually set the master branch ("git checkout -b master
remotes/p4/my_custom_main_branch").
3. I don't know what I missed for that one, but I haven't been able to
get the example for the --branch option to work. It says that after
"git init", we can import a p4 branch with:
git p4 sync --branch=refs/remotes/p4/proj2 //depot/proj2

However, after doing this, followed by "git checkout -b proj2 remotes/p4/proj2", I am unable to properly use "git p4 sync" or "git p4 submit" from this branch, as git complains about a missing refs/remotes/p4/master. Maybe it only works if "git clone" has been used before? But then why have "git init" first?

NB: it's quite possible some of my problems were caused by me not
properly understanding the doc / how git/p4 works. My apologies if
that's the case.
-=- Olivier
Pete Wyckoff· Jan 5, 2013, 21:25 UTC · re: Olivier Delalleau · lore

Re: Suggested improvements to the git-p4 documentation (branch-related)

shish@keba.be wrote on Thu, 03 Jan 2013 15:58 -0500:
> While struggling to get git-p4 to work properly with branches, I
> thought the documentation on http://git-scm.com/docs/git-p4 could use
> some improvements:
Thanks, I definitely appreciate the constructive comments here.
Show 10 quoted lines
> 1. At the end of the "Branch detection" section, the following
> commands are provided (for when you want to explicitly provide branch
> mappings to git-p4):
> 
> git config git-p4.branchList main:branch1
> git p4 clone --detect-branches //depot@all
> 
> The second command should end with a dot (".") because the first
> command only works if you are already in a git-initialized folder.
> Thus I would also suggest to add "git init" as first command to type.
That is confusing.  I'll make it this:
    git init depot
    cd depot
    git config git-p4.branchList main:branch1
    git p4 clone --detect-branches //depot@all .
Show 6 quoted lines
> 2. Even though having a "main" branch is standard in Perforce, it
> would be worth mentioning what happens when you don't: there is a
> message "Could not detect main branch. No checkout/master branch
> created" output by the "git p4 clone" command. However, it will still
> work if you manually set the master branch ("git checkout -b master
> remotes/p4/my_custom_main_branch").

This feels like a bug to me, and indeed I had an old patch series that planned to fix it. Let me knock that into shape, instead of changing the documentation. It will automatically do the checkout step you did.

Show 10 quoted lines
> 3. I don't know what I missed for that one, but I haven't been able to
> get the example for the --branch option to work. It says that after
> "git init", we can import a p4 branch with:
> 
> git p4 sync --branch=refs/remotes/p4/proj2 //depot/proj2
> 
> However, after doing this, followed by "git checkout -b proj2
> remotes/p4/proj2", I am unable to properly use "git p4 sync" or "git
> p4 submit" from this branch, as git complains about a missing
> refs/remotes/p4/master.

Yes, also annoying. I have a failing test case for this, but haven't fixed it yet. The idea is that "git p4 sync --branch=proj2" will sync refs/remotes/p4/proj2. If there is no p4/master, and you don't specify --branch, it will fail with a more useful error message.

For submit, there is code that walks from your current branch back in history until it finds a commit on a known p4 remote branch. This is sort of like the merge-base calculation in git, but restricted to a linear history. I haven't tested that recently, but will add a test and fix it if needed too.

Please do feel welcome to to rearrange or expand the documentation so it makes more sense, if you are so inspired.

		-- Pete
Olivier Delalleau· Jan 7, 2013, 02:00 UTC · re: Pete Wyckoff · lore

Re: Suggested improvements to the git-p4 documentation (branch-related)

2013/1/5 Pete Wyckoff <pw@padd.com>:
Show 24 quoted lines
> shish@keba.be wrote on Thu, 03 Jan 2013 15:58 -0500:
>> While struggling to get git-p4 to work properly with branches, I
>> thought the documentation on http://git-scm.com/docs/git-p4 could use
>> some improvements:
>
> Thanks, I definitely appreciate the constructive comments here.
>
>> 1. At the end of the "Branch detection" section, the following
>> commands are provided (for when you want to explicitly provide branch
>> mappings to git-p4):
>>
>> git config git-p4.branchList main:branch1
>> git p4 clone --detect-branches //depot@all
>>
>> The second command should end with a dot (".") because the first
>> command only works if you are already in a git-initialized folder.
>> Thus I would also suggest to add "git init" as first command to type.
>
> That is confusing.  I'll make it this:
>
>     git init depot
>     cd depot
>     git config git-p4.branchList main:branch1
>     git p4 clone --detect-branches //depot@all .
Sounds good, thanks.
Show 12 quoted lines
>
>> 2. Even though having a "main" branch is standard in Perforce, it
>> would be worth mentioning what happens when you don't: there is a
>> message "Could not detect main branch. No checkout/master branch
>> created" output by the "git p4 clone" command. However, it will still
>> work if you manually set the master branch ("git checkout -b master
>> remotes/p4/my_custom_main_branch").
>
> This feels like a bug to me, and indeed I had an old patch series
> that planned to fix it.  Let me knock that into shape, instead of
> changing the documentation.  It will automatically do the
> checkout step you did.
Sounds good as well.
Show 17 quoted lines
>
>> 3. I don't know what I missed for that one, but I haven't been able to
>> get the example for the --branch option to work. It says that after
>> "git init", we can import a p4 branch with:
>>
>> git p4 sync --branch=refs/remotes/p4/proj2 //depot/proj2
>>
>> However, after doing this, followed by "git checkout -b proj2
>> remotes/p4/proj2", I am unable to properly use "git p4 sync" or "git
>> p4 submit" from this branch, as git complains about a missing
>> refs/remotes/p4/master.
>
> Yes, also annoying.  I have a failing test case for this, but
> haven't fixed it yet.  The idea is that "git p4 sync --branch=proj2"
> will sync refs/remotes/p4/proj2.  If there is no p4/master, and
> you don't specify --branch, it will fail with a more useful error
> message.
Good too!
Show 9 quoted lines
> For submit, there is code that walks from your current branch
> back in history until it finds a commit on a known p4 remote
> branch.  This is sort of like the merge-base calculation in git,
> but restricted to a linear history.  I haven't tested that
> recently, but will add a test and fix it if needed too.
>
>
> Please do feel welcome to to rearrange or expand the
> documentation so it makes more sense, if you are so inspired.

I'm afraid I'm not familiar enough with git documentation to dig into it myself, but anyway that's about what I had for now. I'll send more comments to the mailing list if I have more suggestions in the future.

Thanks for a great tool! :)
-=- Olivier
Junio C Hamano· Feb 23, 2013, 00:42 UTC · re: Olivier Delalleau · lore

Re: Suggested improvements to the git-p4 documentation (branch-related)

Olivier Delalleau <shish@keba.be> writes:
Show 11 quoted lines
> 2013/1/5 Pete Wyckoff <pw@padd.com>:
>> shish@keba.be wrote on Thu, 03 Jan 2013 15:58 -0500:
> ...
>> Please do feel welcome to to rearrange or expand the
>> documentation so it makes more sense, if you are so inspired.
>
> I'm afraid I'm not familiar enough with git documentation to dig into
> it myself, but anyway that's about what I had for now. I'll send more
> comments to the mailing list if I have more suggestions in the future.
>
> Thanks for a great tool! :)

Did anything come out of this thread? If neither of you two are inclined to conclude the discussion with a final patch, I'd appreciate anybody else who does the honors ;-)

We'll be in deep pre-release freeze for a few weeks, so there is no need to rush.

Thanks.
Pete Wyckoff· Feb 23, 2013, 14:48 UTC · re: Junio C Hamano · lore

Re: Suggested improvements to the git-p4 documentation (branch-related)

gitster@pobox.com wrote on Fri, 22 Feb 2013 16:42 -0800:
Show 20 quoted lines
> Olivier Delalleau <shish@keba.be> writes:
> 
> > 2013/1/5 Pete Wyckoff <pw@padd.com>:
> >> shish@keba.be wrote on Thu, 03 Jan 2013 15:58 -0500:
> > ...
> >> Please do feel welcome to to rearrange or expand the
> >> documentation so it makes more sense, if you are so inspired.
> >
> > I'm afraid I'm not familiar enough with git documentation to dig into
> > it myself, but anyway that's about what I had for now. I'll send more
> > comments to the mailing list if I have more suggestions in the future.
> >
> > Thanks for a great tool! :)
> 
> Did anything come out of this thread?  If neither of you two are
> inclined to conclude the discussion with a final patch, I'd
> appreciate anybody else who does the honors ;-)
> 
> We'll be in deep pre-release freeze for a few weeks, so there is no
> need to rush.

Two of Olivier's suggestions were best classified as code, not documentation, bugs. I finished off some ongoing work that fixed those along the way. The third led to a fix to the documentation, 182edef (git p4 doc: fix branch detection example, 2013-01-14), that I added as part of that series.

All of it is in master now, via 801cbd7 (Merge branch 'pw/p4-branch-fixes', 2013-01-21).

I should have commented on this thread too. Thanks for following up!

		-- Pete
Junio C Hamano· Feb 23, 2013, 19:11 UTC · re: Pete Wyckoff · lore

Re: Suggested improvements to the git-p4 documentation (branch-related)

Pete Wyckoff <pw@padd.com> writes:
Show 5 quoted lines
> All of it is in master now, via 801cbd7 (Merge branch
> 'pw/p4-branch-fixes', 2013-01-21).
>
> I should have commented on this thread too.  Thanks for following
> up!
Thanks.

← back to recent threads