stgit truncates binary files to zero length when applying patches

When applying patches and not fast-forwarding, stgit truncates the
binary files to zero length:

  $ cg-init .
  defaulting to local storage area
  Committing initial tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904
  Committed as 73161b6ee428ac8b1c1b16b560c40e13330693d2.

  $ stg init

  $ stg new foo
  Invoking the editor: "emacs .stgit.msg"... done (exit code: 0)

  $ cp /bin/bash .

  $ stg add bash

  $ stg refresh
  Refreshing patch "foo"... done

  $ ls -l
  total 584
  -rwxr-xr-x  1 kha vtech 593304 Nov 15 15:34 bash*

  $ stg pop
  Popping patch "foo"... done
  No patches applied

  $ stg new bar
  Invoking the editor: "emacs .stgit.msg"... done (exit code: 0)

  $ echo bar > bar.txt

  $ stg add bar.txt

  $ stg refresh
  Refreshing patch "bar"... done

  $ stg push foo
  Pushing patch "foo"... done
  Now at patch "foo"

  $ ls -l
  total 4
  -rw-r--r--  1 kha vtech 4 Nov 15 15:34 bar.txt
  -rwxr-xr-x  1 kha vtech 0 Nov 15 15:35 bash*

Without the "bar" patch, popping and then pushing "foo" works as
expected.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

On 15/11/05, Karl Hasselström [off-list ref] wrote:

When applying patches and not fast-forwarding, stgit truncates the
binary files to zero length:

I've never tried binaries with StGIT before. When pushing a patch, if
a merge is needed (like in your case, the base of the foo patch has
changed), StGIT first tries "git-diff-tree | git-apply" for speed
reasons. If this fails, it falls back to a three-way merge.

Unfortunately, git-apply doesn't fail for patches including binary
files and simply creates an empty file. I think git-apply should be
changed to fail to apply this kind of patches.

--
Catalin

On 2005-11-16 11:11:56 +0000, Catalin Marinas wrote:

On 15/11/05, Karl Hasselström [off-list ref] wrote:

quoted

When applying patches and not fast-forwarding, stgit truncates the
binary files to zero length:

I've never tried binaries with StGIT before.

I don't blame you. Binary patches aren't something I normally create
either. It's just that I find stgit patches a good way to logically
structure a largeish change that I'm working on before committing it.
(I could probably accoplish the same thing with one branch instead of
each stgit patch, but then it would be quite a lot of work to manually
push updates through all the branches.)

When pushing a patch, if a merge is needed (like in your case, the
base of the foo patch has changed), StGIT first tries "git-diff-tree
| git-apply" for speed reasons. If this fails, it falls back to a
three-way merge.

Unfortunately, git-apply doesn't fail for patches including binary
files and simply creates an empty file. I think git-apply should be
changed to fail to apply this kind of patches.

Yes, at least if stgit is going to continue to use it like this.
Refusing to handle binary files is somewhat disappointing, but still
OK; agreeing to handle them and then silently wiping them is a bit
less OK. (But don't worry; it is a perfect world, after all, so of
course I had backups. :-)

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

On 16/11/05, Karl Hasselström [off-list ref] wrote:

On 2005-11-16 11:11:56 +0000, Catalin Marinas wrote:

quoted

Unfortunately, git-apply doesn't fail for patches including binary
files and simply creates an empty file. I think git-apply should be
changed to fail to apply this kind of patches.

Yes, at least if stgit is going to continue to use it like this.
Refusing to handle binary files is somewhat disappointing, but still
OK; agreeing to handle them and then silently wiping them is a bit
less OK.

A workaround for this would be to add a config option for StGIT to
always use the three-way merge for pushing patches. The problem with
this is speed since git-diff-tree | git-apply is much faster (and
pretty safe since fuzzy patching is not allowed) and most of the
patches would apply cleanly with only this.

--
Catalin

On 2005-11-16 12:31:27 +0000, Catalin Marinas wrote:

On 16/11/05, Karl Hasselström [off-list ref] wrote:

quoted

On 2005-11-16 11:11:56 +0000, Catalin Marinas wrote:

quoted

Unfortunately, git-apply doesn't fail for patches including
binary files and simply creates an empty file. I think git-apply
should be changed to fail to apply this kind of patches.

Yes, at least if stgit is going to continue to use it like this.
Refusing to handle binary files is somewhat disappointing, but
still OK; agreeing to handle them and then silently wiping them is
a bit less OK.

A workaround for this would be to add a config option for StGIT to
always use the three-way merge for pushing patches. The problem with
this is speed since git-diff-tree | git-apply is much faster (and
pretty safe since fuzzy patching is not allowed) and most of the
patches would apply cleanly with only this.

The proper fix has to be to convince git-apply to either handle
patches with binary files, or to make it fail; in both cases, stgit
will be fine. If the former is somehow intractable or undesirable, and
the latter would break existing callers (and/or inconvenience users),
perhaps it could fail on binary files only when a --text-only flag was
given.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle