git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
* git-p4: default behavior for handling moves?
@ 2019-01-07 12:50 Luke Diamand
  2019-01-08  0:56 ` Mazo, Andrey
  0 siblings, 1 reply; 3+ messages in thread
From: Luke Diamand @ 2019-01-07 12:50 UTC (permalink / raw)
  To: Git Users, Junio C Hamano; +Cc: Chen Bin, Merland Romain, Vitor Antunes

git-p4 can map a "git move" operation to a Perforce "move" operation.
But by default this is disabled. You then end up with a P4 commit
where the file is deleted, and a fresh file is created with the same
contents at the new location at revision #1.

Rename detection gets enabled either with the "-M" option, or with
some config variables, git-p4.detectCopies and git-p4.detectRenames.

I've been tripped up by this, and I actually know about it, and I know
other people have been as well.

Should we switch the default over so that it's enabled by default? I
can't think of any reason why you wouldn't want it enabled.

I think the rename code was first introduced around 2011 by Vitor.

Another option is to add a warning, but people just ignore warnings!

Thanks!
Luke

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: git-p4: default behavior for handling moves?
  2019-01-07 12:50 git-p4: default behavior for handling moves? Luke Diamand
@ 2019-01-08  0:56 ` Mazo, Andrey
  2019-01-09 22:41   ` Vitor Antunes
  0 siblings, 1 reply; 3+ messages in thread
From: Mazo, Andrey @ 2019-01-08  0:56 UTC (permalink / raw)
  To: luke; +Cc: chenbin.sh, git, gitster, merlorom, vitor.hda, Mazo, Andrey

> git-p4 can map a "git move" operation to a Perforce "move" operation.
> But by default this is disabled. You then end up with a P4 commit
> where the file is deleted, and a fresh file is created with the same
> contents at the new location at revision #1.
> 
> Rename detection gets enabled either with the "-M" option, or with
> some config variables, git-p4.detectCopies and git-p4.detectRenames.
> 
> I've been tripped up by this, and I actually know about it, and I know
> other people have been as well.
> 
> Should we switch the default over so that it's enabled by default? I
> can't think of any reason why you wouldn't want it enabled.

I have it enabled in my ~/.gitconfig,
so enabling it by default makes total sense to me.

Regarding potential problems,
I occasionally get a wrong "source" file detection.
(e.g. there was `cp a b ; git add b`, but some other file "c" is selected as the source instead)
Or there is a "copy/move" detected, when there was actually none.
But I've only seen this with quite small files (like a trivial one line shell script) or binaries,
and mostly because I have git-p4.detectCopiesHarder set as well as a pretty aggressive git-p4.detectCopies threshold.
(normally 30%, but down to 10% at times to make sure a copy/move is really detected)

But anyway, enabling both git-p4.detect{Copies,Renames}
with default 50% similarity index makes sense to me.

It's probably worth adding command-line options to override the new to-be defaults though.


A more conservative approach like only enabling git-p4.detectRenames = 70% is an option too.

> 
> I think the rename code was first introduced around 2011 by Vitor.
> 
> Another option is to add a warning, but people just ignore warnings!

When such a warning would be shown?
Just before `p4 submit`?
I think, It might be hard to notice for large changesets.

Thank you,
Andrey

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: git-p4: default behavior for handling moves?
  2019-01-08  0:56 ` Mazo, Andrey
@ 2019-01-09 22:41   ` Vitor Antunes
  0 siblings, 0 replies; 3+ messages in thread
From: Vitor Antunes @ 2019-01-09 22:41 UTC (permalink / raw)
  To: Mazo, Andrey; +Cc: luke, chenbin.sh, git, gitster, merlorom

Hi everyone,

On Tue, 8 Jan 2019 at 00:56, Mazo, Andrey <amazo@checkvideo.com> wrote:
>
> > git-p4 can map a "git move" operation to a Perforce "move" operation.
> > But by default this is disabled. You then end up with a P4 commit
> > where the file is deleted, and a fresh file is created with the same
> > contents at the new location at revision #1.
> >
> > Rename detection gets enabled either with the "-M" option, or with
> > some config variables, git-p4.detectCopies and git-p4.detectRenames.
> >
> > I've been tripped up by this, and I actually know about it, and I know
> > other people have been as well.
> >
> > Should we switch the default over so that it's enabled by default? I
> > can't think of any reason why you wouldn't want it enabled.

The copy/rename detection in git is not so obvious, nor easy to understand.
Also, when you have many similar files it is possible that the source that git
detects is not the real source of the file.
Another thing is that copies are only detected if the source file was modified,
which means that detectCopies without detectCopiesHarder does not make
much sense..

Those were the reasons that I recall at the moment for not having enabled
these settings by default. But that said, I do not oppose in having them
enabled by default.

> I have it enabled in my ~/.gitconfig,
> so enabling it by default makes total sense to me.
>
> Regarding potential problems,
> I occasionally get a wrong "source" file detection.
> (e.g. there was `cp a b ; git add b`, but some other file "c" is selected as the source instead)
> Or there is a "copy/move" detected, when there was actually none.
> But I've only seen this with quite small files (like a trivial one line shell script) or binaries,
> and mostly because I have git-p4.detectCopiesHarder set as well as a pretty aggressive git-p4.detectCopies threshold.
> (normally 30%, but down to 10% at times to make sure a copy/move is really detected)
>
> But anyway, enabling both git-p4.detect{Copies,Renames}
> with default 50% similarity index makes sense to me.
>
> It's probably worth adding command-line options to override the new to-be defaults though.
>
>
> A more conservative approach like only enabling git-p4.detectRenames = 70% is an option too.

I'm using detectCopies=89% because copied/moved files, in general,
will have a very high similarity
level and I don't want to have new files with similar contents
(imagine test cases, for example) to be
automatically detected as copies.

> > I think the rename code was first introduced around 2011 by Vitor.

Time flies!
I was using a much more complex P4 setup at the time that relied a lot
on renames and copies, as
well as branches. I have a much more linear setup now.

Best regards,
Vitor

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-07 12:50 git-p4: default behavior for handling moves? Luke Diamand
2019-01-08  0:56 ` Mazo, Andrey
2019-01-09 22:41   ` Vitor Antunes

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox