New directory lost by git am

New directory lost by git am

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

I applied a patch with git am that adds a new source file to a new
directory, and later noticed that file was missing from the commit.
It seems that git am fails to add the new file/directory to the index.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBCgAGBQJTFpCjAAoJEI5FoCIzSKrw1CsH/1E/0Wgs3RtXPLqWbwVoFy+U
Bc7dW7TBmb8EScC+3DedI4u9ryjZigjbsnBg1Y8V/gEtmUSmvt1e8CWTdvMLQpvx
bnasL4uia/CBOg/aZkJ1iEBiHA3sUi9Es4FqoHbuGBn0bkDrA2NQvt3bCqNf6n8H
PCeWx/qb8+F4niI0I8T5ASeqOHMxxSegHvlGezl6XZoGHa5SeLRrg7JtW3ZoWKCO
q6GRzR6dV4FWJckfajUo34IUQNS4YA7wLpmC3PVUn3+EgF+affAEigjVWGRWdf2k
cuaNu6hUAuD/2EHhCt6YP+ubV+FYiU86QOvmVifVpH1Apd29Fw4Kqnvyq2zJVC0=
=hXsK
-----END PGP SIGNATURE-----

Re: New directory lost by git am

[Chris Packham]

Hi,

On 05/03/14 15:49, Phillip Susi wrote:
> I applied a patch with git am that adds a new source file to a new
> directory, and later noticed that file was missing from the commit.
> It seems that git am fails to add the new file/directory to the index.
> 

Could you provide a few more details such as your git version (git
--version) and an example of the failure. I've tried to reproduce the
problem based on the description provided but everything seems to work
as expected for me.

  git --version
    git version 1.9.0
  mkdir test && cd test && git init
  echo "hello world" >a.txt
  git add a.txt
  git commit -m"Initial commit"
  git checkout -b temp
  mkdir b
  echo "lorem ipsum" >b/b.txt
  git add b/b.txt
  git commit -m"Add b/b.txt"
  ls -R
    .:
    a.txt  b

    ./b:
    b.txt
  git checkout master
  git format-patch temp -1 --stdout | git am
  ls -R
    .:
    a.txt  b

    ./b:
    b.txt

Re: New directory lost by git am

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 03/04/2014 10:08 PM, Chris Packham wrote:
> Could you provide a few more details such as your git version (git 
> --version) and an example of the failure. I've tried to reproduce
> the problem based on the description provided but everything seems
> to work as expected for me.

Version 1.8.3.2.

> git --version git version 1.9.0 mkdir test && cd test && git init 
> echo "hello world" >a.txt git add a.txt git commit -m"Initial
> commit" git checkout -b temp mkdir b echo "lorem ipsum" >b/b.txt 
> git add b/b.txt git commit -m"Add b/b.txt" ls -R .: a.txt  b
> 
> ./b: b.txt git checkout master git format-patch temp -1 --stdout |
> git am ls -R .: a.txt  b
> 
> ./b: b.txt
> 

You are reapplying the patch while it is already applied.  Do a reset
HEAD~1 --hard, and git clean -x -f -d before git am.  I didn't notice
the missing file myself for some time because it is left in the
working tree, just not added to the index and included in the commit.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBCgAGBQJTFphoAAoJEI5FoCIzSKrwx78H/iTLvtMVb2hmn2g2YDQuJWe3
nENrqlRNDF11YHpA9c7chxepcuP2CZaZjoXv45aCQG9Wx9XJyKPIbauhwqIIVUjR
VYDORdtpn8u3Pf3WWyHYW+MEoupYyni4VYENVSjKnV6sLT951TuYI+4paHWat3lq
/at9UkLy4d39hj2P/6M+voBbKWzblBZzP31lH6OY/Mno2zhh4eQChhsnZYPQ/Hfn
REAeyB4WsLCjnPz+uEkOcWaEVVh+BwNU1UmK/tX+rzhBsaRzhDY5IIWTL9dfkD/z
Af86IUSKdTjnMq7CTmVAmlxAfHXF0bgtlybrVY2Sdc8R/CqmWCz6USyKdUxgLIk=
=Z3Z/
-----END PGP SIGNATURE-----

Re: New directory lost by git am

[Chris Packham]

On 05/03/14 16:22, Phillip Susi wrote:
> On 03/04/2014 10:08 PM, Chris Packham wrote:
>> Could you provide a few more details such as your git version (git 
>> --version) and an example of the failure. I've tried to reproduce
>> the problem based on the description provided but everything seems
>> to work as expected for me.
> 
> Version 1.8.3.2.
> 
>> git --version git version 1.9.0 mkdir test && cd test && git init 
>> echo "hello world" >a.txt git add a.txt git commit -m"Initial
>> commit" git checkout -b temp mkdir b echo "lorem ipsum" >b/b.txt 
>> git add b/b.txt git commit -m"Add b/b.txt" ls -R .: a.txt  b
>>
>> ./b: b.txt git checkout master git format-patch temp -1 --stdout |
>> git am ls -R .: a.txt  b
>>
>> ./b: b.txt
>>
> 
> You are reapplying the patch while it is already applied.

My example is creating a commit on the "temp" branch then applying it to
the "master" branch using git am.

> Do a reset
> HEAD~1 --hard, and git clean -x -f -d before git am.  I didn't notice
> the missing file myself for some time because it is left in the
> working tree, just not added to the index and included in the commit.
> 

Regardless of reproducing the issue a quick glance at the Release notes
for 1.8.3.3 the following sticks out:

Fixes since v1.8.3.2
--------------------

 * "git apply" parsed patches that add new files, generated by programs
   other than Git, incorrectly.  This is an old breakage in v1.7.11.

Does that sound like your problem? If you can I'd suggest updating,
ideally to the recent 1.9.0 release but if you're feeling conservative
try 1.8.3.4.

Re: New directory lost by git am

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 3/5/2014 3:10 AM, Chris Packham wrote:
> My example is creating a commit on the "temp" branch then applying
> it to the "master" branch using git am.
> 
>> Do a reset HEAD~1 --hard, and git clean -x -f -d before git am.
>> I didn't notice the missing file myself for some time because it
>> is left in the working tree, just not added to the index and
>> included in the commit.
>> 

Right... so the file is left in the directory, even though it is not
checked in.  A git status should show it is an unknown file, and a
clean should remove it.

> Regardless of reproducing the issue a quick glance at the Release
> notes for 1.8.3.3 the following sticks out:
> 
> Fixes since v1.8.3.2 --------------------
> 
> * "git apply" parsed patches that add new files, generated by
> programs other than Git, incorrectly.  This is an old breakage in
> v1.7.11.
> 
> Does that sound like your problem? If you can I'd suggest
> updating, ideally to the recent 1.9.0 release but if you're feeling
> conservative try 1.8.3.4.

Vaguely, except for the "other than git" part.  This patch was
generated by git-format-patch ( I didn't think apply handled patches
that weren't ).


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTFzQiAAoJEI5FoCIzSKrwXEMH/iQdFdAApGnFCyLGA4l87d4M
ITLtyL632gHA39KnlEqtTc6TgFjQMrV3m8s9TeR6DiEI9qGvNvnP2E4JBFORZprk
RSJoCa9qMuAYOtSEtwrzbhMZpBN7hAZeJ7txP2KwZiGXoWjr4RSawjViQHhnXQEP
L9QMmwjWJBwZE/eklYg8W+Ov987uribGTgOL8Wx1iMME2C88VfyCWtg1ClTz3aEh
UeyAvyLxSA+YvS4xg+nUBAxXX8bFI0g53Yjf3Lt/1/EzdO67sH7CRChR67BmANWZ
NBoxBqenN6/qg0rfRojOnjGTtRpAJX48dgnEHulUJrixrmqBYXAbi8dNVW8KkiM=
=tzX5
-----END PGP SIGNATURE-----

Re: New directory lost by git am

[Jeff King]

On Wed, Mar 05, 2014 at 09:26:43AM -0500, Phillip Susi wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 3/5/2014 3:10 AM, Chris Packham wrote:
> > My example is creating a commit on the "temp" branch then applying
> > it to the "master" branch using git am.
> > 
> >> Do a reset HEAD~1 --hard, and git clean -x -f -d before git am.
> >> I didn't notice the missing file myself for some time because it
> >> is left in the working tree, just not added to the index and
> >> included in the commit.
> >> 
> 
> Right... so the file is left in the directory, even though it is not
> checked in.  A git status should show it is an unknown file, and a
> clean should remove it.

I don't think those steps are necessary for Chris's example. When he
switches back to the master branch, git removes the subdirectory (the
file is tracked in "temp" but not "master", so we remove it when
switching branches, and then the directory is empty, so we clean it up,
too). You can verify with an extra "ls" after the checkout but before
the "am".

> > * "git apply" parsed patches that add new files, generated by
> > programs other than Git, incorrectly.  This is an old breakage in
> > v1.7.11.
> > 
> > Does that sound like your problem? If you can I'd suggest
> > updating, ideally to the recent 1.9.0 release but if you're feeling
> > conservative try 1.8.3.4.
> 
> Vaguely, except for the "other than git" part.  This patch was
> generated by git-format-patch ( I didn't think apply handled patches
> that weren't ).

I can't get Chris's script to fail on any version of git. Can you show
us an example of a patch that does not behave (or better yet, a
reproduction recipe to generate the patch with "format-patch")?

-Peff

Re: New directory lost by git am

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 3/5/2014 11:34 AM, Jeff King wrote:
> I don't think those steps are necessary for Chris's example. When
> he switches back to the master branch, git removes the subdirectory
> (the file is tracked in "temp" but not "master", so we remove it
> when switching branches, and then the directory is empty, so we
> clean it up, too). You can verify with an extra "ls" after the
> checkout but before the "am".

Right.

>>> * "git apply" parsed patches that add new files, generated by 
>>> programs other than Git, incorrectly.  This is an old breakage
>>> in v1.7.11.
>>> 
>>> Does that sound like your problem? If you can I'd suggest 
>>> updating, ideally to the recent 1.9.0 release but if you're
>>> feeling conservative try 1.8.3.4.
>> 
>> Vaguely, except for the "other than git" part.  This patch was 
>> generated by git-format-patch ( I didn't think apply handled
>> patches that weren't ).
> 
> I can't get Chris's script to fail on any version of git. Can you
> show us an example of a patch that does not behave (or better yet,
> a reproduction recipe to generate the patch with "format-patch")?

AHA!  It requires a conflict.  There were simple conflicts in the NEWS
file so I applied the patch with git am --reject and fixed up the
NEWS, and ran git am --resolved.  The git am --reject fails to add the
new directory to the index.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTF1UOAAoJEI5FoCIzSKrwTD4H/35pUf8DFsbwPIVVQi+8I8e3
5NMHwQrHK3TPbZigVPBgVfwRCtOAxX656BPhninfhix99HWs00W5zGaFDwkymRNp
87EeU3LVcIjapqijszw9AqwBLvfm9uzXEus964hShCJVOmKBezQfl6Mvcrkn5Na1
UchJLkRzEoi6VUyUso8FH0xpL7JyjF08H19dtvXoUbrvrXYuN1Ys3UMBHXVEVdi+
5O924lo4+psgdjGZ3HUpclYRbKO0LS5IVMCxFRw5Q+EfARJQ7NXzv/csRXIKyms7
roCQqmQnnem71GHx6SQaepnY5pKuEnmmDaqXbCOqZdpyfo1CB7SFJDq/VXrbLyw=
=zS2r
-----END PGP SIGNATURE-----

Re: New directory lost by git am

[Jeff King]

On Wed, Mar 05, 2014 at 11:47:12AM -0500, Phillip Susi wrote:

> > I can't get Chris's script to fail on any version of git. Can you
> > show us an example of a patch that does not behave (or better yet,
> > a reproduction recipe to generate the patch with "format-patch")?
> 
> AHA!  It requires a conflict.  There were simple conflicts in the NEWS
> file so I applied the patch with git am --reject and fixed up the
> NEWS, and ran git am --resolved.  The git am --reject fails to add the
> new directory to the index.

Thanks, I can reproduce here. I do not think it has anything to do with
being in a subdirectory; any new file does not get added to the index.
In fact, I do not think we update the index at all with "--reject". For
example, try this:

    git init repo &&
    cd repo &&

    echo base >conflict &&
    echo base >modified &&
    git add . &&
    git commit -m base &&

    echo master >conflict &&
    git add . &&
    git commit -m master &&

    git checkout -b other HEAD^ &&
    echo other >conflict &&
    echo other >modified &&
    echo other >new &&
    git add . &&
    git commit -m other &&

    git checkout master &&
    git format-patch other -1 --stdout >patch &&
    git am --reject patch

Running "git status -s" shows:

   M modified
   ?? conflict.rej
   ?? new
   ?? patch

We apply the changes to "modified" and "new" to the working tree, but we
do not stage anything in the index. I suspect this is because our
invocation of "apply --index" (which is what is doing the real work with
"--reject" here) bails before touching the index. In theory it should be
able to update the index for files that applied cleanly and leave the
other ones alone.

But I have not thought hard about it, so maybe there is a good reason
not to (it is a little weird just because the resulting index is a
partial application of the patch).  The "am -3" path does what you want
here, but it is much simpler: it knows it can represent the 3-way
conflict in the index. So the index represents the complete state of the
patch application at the end, including conflicts.

-Peff

Re: New directory lost by git am

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 3/5/2014 12:13 PM, Jeff King wrote:
> We apply the changes to "modified" and "new" to the working tree,
> but we do not stage anything in the index. I suspect this is
> because our invocation of "apply --index" (which is what is doing
> the real work with "--reject" here) bails before touching the
> index. In theory it should be able to update the index for files
> that applied cleanly and leave the other ones alone.

Yikes, that's really bad.

> But I have not thought hard about it, so maybe there is a good
> reason not to (it is a little weird just because the resulting
> index is a partial application of the patch).  The "am -3" path
> does what you want here, but it is much simpler: it knows it can
> represent the 3-way conflict in the index. So the index represents
> the complete state of the patch application at the end, including
> conflicts.

yes, but -3 fails if it can't find the parent blobs.



-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTF20HAAoJEI5FoCIzSKrwFfQIAJfPmplu7zskercvjnuZGCle
ccTzK0rYtrwQn/78Vrbc6kqcrQvbvtrqUMN4/ILJ5xeaO80Gzzz8pchBPNN8khhY
VBQiWUOrKzBH1vszveneva+gFUrLIWk2KI6T8lGTnYulvRVe38WRAwr/8qEClPX6
hUnYChM17WE+KTV39ezA6ww9ZAyOX7EHq87PJp5nVgBB4HkmkDmccfxYTFNB4FGg
PPqun8g0Fyytd+Qrsk2M5L6NsPUXi32wIt8EWcyPwU6QrQgKbuWK7QlVkcPPzecM
eHifKm8V1V0VKudm3S8jbaUDG2KnLIdMveXu/e9Hn7+YgDQh9zM1m7f+NVJDvjk=
=NAe9
-----END PGP SIGNATURE-----

Re: New directory lost by git am

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> But I have not thought hard about it, so maybe there is a good reason
> not to (it is a little weird just because the resulting index is a
> partial application of the patch).

Originally ".rej" was a deliberate attempt to be "not very Git but
more like 'patch'", so I wouldn't be surprised if the combination
between "--reject" and "--index" did not work, in the sense that we
did not add such a partial change to the index.

I do not offhand think of a reason to forbid the combination,
though, as long as we make sure that "git apply --index --reject"
still exits with failure to prevent a partial application to be
auto-committed.