From: Junio C Hamano <hidden> Date: 2016-06-15 23:08:22
Amadeusz Żołnowski [off-list ref] writes:
Junio C Hamano [off-list ref] writes:
quoted
I wonder if it makes sense to just silently allowing submit without
rebasing, and without telling the user how to clean it up.
It is a good point. Maybe submit shouldn't be allowed by default at all
From bare repository. What do you think about following approach? If
core.base is true, then if git-p4.submitFromBare is also true then
submit without performing proper rebase, but update master ref from
p4/master. If core.base is true, but git-p4.submitFromBare is false,
abort immediately. Submitting from bare repository would require aware
decision from a user.
(I think you meant "bare" where you wrote "base" in the above).
As a non user of git-p4, the above sounds a safe and sensible
approach, but I'd like to hear from Luke, who has been active in
this area and is far more expert than I am ;-)
Thanks.
From: Luke Diamand <hidden> Date: 2016-06-15 23:08:22
On 23 February 2016 at 06:59, Junio C Hamano [off-list ref] wrote:
Amadeusz Żołnowski [off-list ref] writes:
quoted
Junio C Hamano [off-list ref] writes:
quoted
I wonder if it makes sense to just silently allowing submit without
rebasing, and without telling the user how to clean it up.
It is a good point. Maybe submit shouldn't be allowed by default at all
From bare repository. What do you think about following approach? If
core.base is true, then if git-p4.submitFromBare is also true then
submit without performing proper rebase, but update master ref from
p4/master. If core.base is true, but git-p4.submitFromBare is false,
abort immediately. Submitting from bare repository would require aware
decision from a user.
(I think you meant "bare" where you wrote "base" in the above).
As a non user of git-p4, the above sounds a safe and sensible
approach, but I'd like to hear from Luke, who has been active in
this area and is far more expert than I am ;-)
Sorry, I've been travelling so not been able to reply earlier. Jetlag
has now kicked-in, and 4am is the perfect time to reply to this!
I think it would help to understand why we're doing a git-p4 submit
from a bare repo in the first place.
The usual workflow is you have a normal repo and submit from that.
I'm guessing that the reason for using a bare repo is so that changes
can be pushed to it from several different git repos. This then saves
doing the initial git-p4 clone multiple times.
As Amadeusz pointed out, the submit fails:
$ git p4 submit --git-dir=/path/to/bare/repo
...
...
fatal: This operation must be run in a work tree
Some files in your working directory are modified and different than
what is in your index. You can use git update-index <filename> to
bring the index up-to-date or stash away all your changes with git
stash.
If this had actually worked, I think the next thing I would want to do
is to rebase one or more branches in the bare repo against p4/master.
I don't think there's any way that git-p4 can work out which branches
would be rebased, and nor should it.
I think the approach of using a submitFromBare config variable to
force the user to make a choice feels a bit bogus, since they clearly
*want* to submit from this bare repo, as otherwise they wouldn't have
done "git-p4 submit" in the first place.
It might make sense to have a command-line or config option
("--skip-rebase" ?) to tell "submit" to only do the submit part, and
skip the rebase stage (and get the rebase stage to give a more useful
error message on a bare repo when the option isn't used). That would
then mean that git-p4 does not have to know if it's running in a bare
repo or not, and the submit-without-rebase functionality is available
to people doing other different things not involving bare repos (which
we haven't though of yet) but still requiring submit without rebase.
Luke
I'm guessing that the reason for using a bare repo is so that changes
can be pushed to it from several different git repos. This then saves
doing the initial git-p4 clone multiple times.
I have created a Git repository acting as a bridge between Perforce and
pure Git repos. Changes pushed to master branch on this bridge repo get
submitted to Perforce repository (referenced via remote p4/master).
If this had actually worked, I think the next thing I would want to do
is to rebase one or more branches in the bare repo against p4/master.
I don't think there's any way that git-p4 can work out which branches
would be rebased, and nor should it.
It actually has all information needed. It submits commits from a given
branch to a branch specified with --branch option (or default p4
remote). When submitting from a non-bare repo git-p4 has the same set of
information: the current branch and a branch specified with --branch (or
default p4 remote).
I think the approach of using a submitFromBare config variable to
force the user to make a choice feels a bit bogus, since they clearly
*want* to submit from this bare repo, as otherwise they wouldn't have
done "git-p4 submit" in the first place.
I agree, a good point.
It might make sense to have a command-line or config option
("--skip-rebase" ?) to tell "submit" to only do the submit part, and
skip the rebase stage (and get the rebase stage to give a more useful
error message on a bare repo when the option isn't used). That would
then mean that git-p4 does not have to know if it's running in a bare
repo or not, and the submit-without-rebase functionality is available
to people doing other different things not involving bare repos (which
we haven't though of yet) but still requiring submit without rebase.
While having additional --skip-rebase is a good idea, having git-p4
doing rebase would be more elegant for those who actually use GitP4 in
bare repository. In message 87fuwnd4u7.fsf@freja.aidecoe.name I have
described how state of branches changes during submit. It clearly shows
that in case of a bare repository it ends up in undesired state.
To simplify things, why not just update ref during submit from bare
repository? As you have pointed out, if user invokes submit in this
context he/she actually wants to submit from bare repo and probably
knows what he/she is doing - especially if he/she reads man page. (-:
Kind regards,
--
Amadeusz Żołnowski
From: Luke Diamand <hidden> Date: 2016-06-15 23:08:31
On 23 February 2016 at 20:56, Amadeusz Żołnowski [off-list ref] wrote:
To simplify things, why not just update ref during submit from bare
repository? As you have pointed out, if user invokes submit in this
context he/she actually wants to submit from bare repo and probably
knows what he/she is doing - especially if he/she reads man page. (-:
Will it do something sensible if anything goes wrong?
I'm thinking about what happens if you submit and one of the commits
fails to go to P4 due to a merge conflict.
But I guess just doing an update-ref could be an OK thing to do.
Without actually trying it out for real though I can't be sure. It
might make sense to get some practical experience of how this works
out.
Luke
On 23 February 2016 at 20:56, Amadeusz Żołnowski [off-list ref] wrote:
quoted
To simplify things, why not just update ref during submit from bare
repository? As you have pointed out, if user invokes submit in this
context he/she actually wants to submit from bare repo and probably
knows what he/she is doing - especially if he/she reads man page. (-:
Will it do something sensible if anything goes wrong?
I'm thinking about what happens if you submit and one of the commits
fails to go to P4 due to a merge conflict.
Handling failures in this case is a bit tricky, indeed.
But I guess just doing an update-ref could be an OK thing to do.
Without actually trying it out for real though I can't be sure. It
might make sense to get some practical experience of how this works
out.
True. For now I have these cases covered by wrapper scripts. The minimum
I need from git-p4 is just not to fail on git submit from bare
repository which is covered by patch I have submitted. If I get my
solution enough testing, I'd think of transforming it into patch for
git-p4.py as well.
--
Amadeusz Żołnowski
From: Luke Diamand <hidden> Date: 2016-06-15 23:08:32
On 28 February 2016 at 20:46, Amadeusz Żołnowski [off-list ref] wrote:
True. For now I have these cases covered by wrapper scripts. The minimum
I need from git-p4 is just not to fail on git submit from bare
repository which is covered by patch I have submitted. If I get my
solution enough testing, I'd think of transforming it into patch for
git-p4.py as well.
Could you change the patch to add a command-line option to suppress
the rebase? I think this would be a bit more obvious: instead of
having some special magical behaviour kick-in on a bare repo, git-p4
just does what it's told on the command-line.
It means that if we find another situation where we don't want to
rebase, we don't have an ever-growing list of special-case
circumstances, which could become hard to make sense of in future.
Instead, the user (who hopefully knows better) just tells git-p4 what
to do.
Thanks!
Luke