Re: Do we want 'svn patch' to be able to add empty files?

2010-09-26 Thread Daniel Näslund
On Wed, Sep 01, 2010 at 06:37:08PM +0100, Julian Foad wrote:
 Daniel Näslund wrote:
  The question is: Do we want 'svn patch to be able to add empty files.

 If add an empty file is a well defined operation in Git-diff syntax,
 then yes, it would be good to support it.  As well as delete this empty
 file and any other valid Subversion operations that aren't yet
 supported.

Support for deleting empty files was added in r1001493. 

There's a lot of other interresting stuff in this thread (adding support
for dirs in the git format and properly applying symlinks). Will take a
stab at those in the close future.

Daniel


Re: Do we want 'svn patch' to be able to add empty files?

2010-09-04 Thread Branko Čibej
 On 04.09.2010 02:44, Augie Fackler wrote:

 On Sep 3, 2010, at 7:10 AM, Daniel Näslund wrote:

 On Fri, Sep 03, 2010 at 12:18:37PM +0200, Branko Čibej wrote:
 On 02.09.2010 10:50, Branko Čibej wrote:
 Hmm, this is interesting. :) Git faithfully (blindly?) interprets Unix
 permission bits, whiles SVN faithfully (blindly?) interprets the
 contents of special files ... I wonder if svn patch does the right
 thing here?

 Anyway, for the sake of interoperability, we'd have to emit and parse
 the git format for symlinks. Not that I'm too amused by the idea that
 git probably just does a chmod on the new file without thinking about
 it, but hey, All the World is Linux, right? :)

 Did some testing ... apparently git apply completely ignores the
 permission bits new file mode ... line, at least I haven't been able
 to force it to do anything with them.

 From builtin/apply::try_create_file() in the git source code:

 fd = open(path, O_CREAT | O_EXCL | O_WRONLY, (mode  0100) ? 0777 :
 0666);
  if (fd  0)
return -1;

 Git only checks for the executable bit, AFAIK.

 Correct, git and hg only store files as 0644 or 0755. Everything else
 gets handled by the umask on the user's machine.

Nice ... that's equivalent to svn:executable. And see, I was right about
the magic numbers. :)

-- Brane



Re: Do we want 'svn patch' to be able to add empty files?

2010-09-04 Thread Branko Čibej
 On 04.09.2010 02:46, Augie Fackler wrote:

 On Sep 2, 2010, at 4:03 AM, Daniel Näslund wrote:

 $ svn diff --git
 Index: empty
 ===
 diff --git a/trunk/empty b/trunk/empty
 new directory mode 10644

 I'd recommend testing this against hg/git before using it, but it
 should operate correctly in hg given my recollection of the diff parser.

Come to think of it, it should be at least

new directory mode 10755

or there might be trouble brewing downstream if anyone ever interprets
the execute directory bit in the mode mask ...

-- Brane


Re: Do we want 'svn patch' to be able to add empty files?

2010-09-03 Thread Branko Čibej
 On 02.09.2010 10:50, Branko Čibej wrote:
  On 02.09.2010 10:27, Daniel Shahaf wrote:
 Daniel Näslund wrote on Thu, Sep 02, 2010 at 07:13:00 +0200:
 On Wed, Sep 01, 2010 at 06:37:08PM +0100, Julian Foad wrote:
 This may be off topic, but I'm wondering whether Git has defined such
 operations on directories fully or at all, since it doesn't version them
 explicitly.  I mean, can you tell the difference between add empty file
 A and add empty dir A?  I could go and look, but haven't time today.
 If yes, great; if it doesn't, we'll have to invent syntax extensions to
 do it.  (I'm recalling that the goal of this work is we want Subversion
 diffs to be able to support all valid Subversion changes, and we chose
 Git format as a basis for supporting that.  We don't want to constrain
 ourselves to only the operations that Git supports.)
 Not supported at the moment:

 $ svn mkdir X
 A   X
 $ svn status
 AX
 $ svn diff
 $ svn diff --git
 $

 Suggestion:

 $ svn diff --git
 Index: empty
 ===
 diff --git a/trunk/empty b/trunk/empty
 new directory mode 10644
 IIRC trailing slashes on empty/ were suggested on IRC, what was the
 conclusion about that?

 E.g., just changing the 'new file mode 10644' line to mention directory
 instead. Haven't investigate what changes would be needed in the diff
 editor.
 By the way, are we just influenced by Git's format, or are we looking
 for some degree of interoperability?  Consider adding a symlink:

 [[[
 ### with git (in $wcroot/trunk/):
 diff --git a/trunk/bar b/trunk/bar
 new file mode 12
 index 000..1910281
 --- /dev/null
 +++ b/trunk/bar
 @@ -0,0 +1 @@
 +foo
 \ No newline at end of file

 ### with svn (in $wcroot/trunk/):
 Index: bar
 ===
 diff --git a/trunk/bar b/trunk/bar
 new file mode 10644
 --- /dev/null   (revision 0)
 +++ b/trunk/bar (working copy)
 @@ -0,0 +1 @@
 +link foo
 \ No newline at end of file

 Property changes on: trunk/bar
 ___
 Added: svn:special
 ## -0,0 +1 ##
 +*
 ]]]
 Hmm, this is interesting. :) Git faithfully (blindly?) interprets Unix
 permission bits, whiles SVN faithfully (blindly?) interprets the
 contents of special files ... I wonder if svn patch does the right
 thing here?

 Anyway, for the sake of interoperability, we'd have to emit and parse
 the git format for symlinks. Not that I'm too amused by the idea that
 git probably just does a chmod on the new file without thinking about
 it, but hey, All the World is Linux, right? :)

Did some testing ... apparently git apply completely ignores the
permission bits new file mode ... line, at least I haven't been able
to force it to do anything with them.

-- Brane


Re: Do we want 'svn patch' to be able to add empty files?

2010-09-03 Thread Daniel Näslund
On Fri, Sep 03, 2010 at 12:18:37PM +0200, Branko Čibej wrote:
  On 02.09.2010 10:50, Branko Čibej wrote:
  Hmm, this is interesting. :) Git faithfully (blindly?) interprets Unix
  permission bits, whiles SVN faithfully (blindly?) interprets the
  contents of special files ... I wonder if svn patch does the right
  thing here?
 
  Anyway, for the sake of interoperability, we'd have to emit and parse
  the git format for symlinks. Not that I'm too amused by the idea that
  git probably just does a chmod on the new file without thinking about
  it, but hey, All the World is Linux, right? :)
 
 Did some testing ... apparently git apply completely ignores the
 permission bits new file mode ... line, at least I haven't been able
 to force it to do anything with them.

From builtin/apply::try_create_file() in the git source code:

 fd = open(path, O_CREAT | O_EXCL | O_WRONLY, (mode  0100) ? 0777 : 0666);
  if (fd  0)
return -1;

Git only checks for the executable bit, AFAIK.

Daniel


Re: Do we want 'svn patch' to be able to add empty files?

2010-09-03 Thread Augie Fackler


On Sep 3, 2010, at 7:10 AM, Daniel Näslund wrote:


On Fri, Sep 03, 2010 at 12:18:37PM +0200, Branko Čibej wrote:

On 02.09.2010 10:50, Branko Čibej wrote:
Hmm, this is interesting. :) Git faithfully (blindly?) interprets  
Unix

permission bits, whiles SVN faithfully (blindly?) interprets the
contents of special files ... I wonder if svn patch does the right
thing here?

Anyway, for the sake of interoperability, we'd have to emit and  
parse
the git format for symlinks. Not that I'm too amused by the idea  
that
git probably just does a chmod on the new file without thinking  
about

it, but hey, All the World is Linux, right? :)


Did some testing ... apparently git apply completely ignores the
permission bits new file mode ... line, at least I haven't been  
able

to force it to do anything with them.


From builtin/apply::try_create_file() in the git source code:

fd = open(path, O_CREAT | O_EXCL | O_WRONLY, (mode  0100) ? 0777 :  
0666);

 if (fd  0)
   return -1;

Git only checks for the executable bit, AFAIK.


Correct, git and hg only store files as 0644 or 0755. Everything else  
gets handled by the umask on the user's machine.




Daniel




Re: Do we want 'svn patch' to be able to add empty files?

2010-09-02 Thread Daniel Shahaf
(Thanks for the examples.  I suppose next time I should try to run
'touch foo; $svn add foo; $svn diff --git' myself...)

Daniel Näslund wrote on Thu, Sep 02, 2010 at 07:05:41 +0200:
 On Wed, Sep 01, 2010 at 10:54:08PM +0300, Daniel Shahaf wrote:
  Daniel Näslund wrote on Wed, Sep 01, 2010 at 11:28:51 +0200:
   (This started out as me trying to apply added paths using the information
   from a patch file in the git diff format. The only that I could come up
   with where an add could not be detected by just looking at the regular
   unidiff headers  was adding an empty file (it has no unidiff headers).
   If anyone has any other cases, please let me know.)
   
  
  How does a diff adding an empty file look?
 
 Like this:
 
 Index: empty
 ===
 diff --git a/trunk/empty b/trunk/empty
 new file mode 10644
 

So, it boils down to having to recognize /^new file mode/ (even though
there are no following /^(---|+++)/ lines), right?

I've never been inside the patch code, I don't know how easy/tricky it
would be to add this.

 Note that we allow empty files to be created for regular diffs too if
 they have property changes. This patch will create an empty file with
 property 'foo' set on it:
 
 Index: empty
 ===
 
 Property changes on: empty
 ___
 Added: foo
 ## -0,0 +1 ##
 +value

So this implicitly creates the file if it doesn't exist already; in
other words, we do not distinguish setting a property on an existing
file (without content changes) from adding a file with properties.
Would it be better to make a distinction --- for example, by generating
a /^new file/ line in the latter case?  (that would be explicit and more
friendly to non-property-aware tools)



Re: Do we want 'svn patch' to be able to add empty files?

2010-09-02 Thread Daniel Shahaf
Daniel Näslund wrote on Thu, Sep 02, 2010 at 07:13:00 +0200:
 On Wed, Sep 01, 2010 at 06:37:08PM +0100, Julian Foad wrote:
  This may be off topic, but I'm wondering whether Git has defined such
  operations on directories fully or at all, since it doesn't version them
  explicitly.  I mean, can you tell the difference between add empty file
  A and add empty dir A?  I could go and look, but haven't time today.
  If yes, great; if it doesn't, we'll have to invent syntax extensions to
  do it.  (I'm recalling that the goal of this work is we want Subversion
  diffs to be able to support all valid Subversion changes, and we chose
  Git format as a basis for supporting that.  We don't want to constrain
  ourselves to only the operations that Git supports.)
 
 Not supported at the moment:
 
 $ svn mkdir X
 A   X
 $ svn status
 AX
 $ svn diff
 $ svn diff --git
 $
 
 Suggestion:
 
 $ svn diff --git
 Index: empty
 ===
 diff --git a/trunk/empty b/trunk/empty
 new directory mode 10644

IIRC trailing slashes on empty/ were suggested on IRC, what was the
conclusion about that?

 
 E.g., just changing the 'new file mode 10644' line to mention directory
 instead. Haven't investigate what changes would be needed in the diff
 editor.

By the way, are we just influenced by Git's format, or are we looking
for some degree of interoperability?  Consider adding a symlink:

[[[
### with git (in $wcroot/trunk/):
diff --git a/trunk/bar b/trunk/bar
new file mode 12
index 000..1910281
--- /dev/null
+++ b/trunk/bar
@@ -0,0 +1 @@
+foo
\ No newline at end of file

### with svn (in $wcroot/trunk/):
Index: bar
===
diff --git a/trunk/bar b/trunk/bar
new file mode 10644
--- /dev/null   (revision 0)
+++ b/trunk/bar (working copy)
@@ -0,0 +1 @@
+link foo
\ No newline at end of file

Property changes on: trunk/bar
___
Added: svn:special
## -0,0 +1 ##
+*
]]]


Re: Do we want 'svn patch' to be able to add empty files?

2010-09-02 Thread Branko Čibej
 On 02.09.2010 10:27, Daniel Shahaf wrote:
 Daniel Näslund wrote on Thu, Sep 02, 2010 at 07:13:00 +0200:
 On Wed, Sep 01, 2010 at 06:37:08PM +0100, Julian Foad wrote:
 This may be off topic, but I'm wondering whether Git has defined such
 operations on directories fully or at all, since it doesn't version them
 explicitly.  I mean, can you tell the difference between add empty file
 A and add empty dir A?  I could go and look, but haven't time today.
 If yes, great; if it doesn't, we'll have to invent syntax extensions to
 do it.  (I'm recalling that the goal of this work is we want Subversion
 diffs to be able to support all valid Subversion changes, and we chose
 Git format as a basis for supporting that.  We don't want to constrain
 ourselves to only the operations that Git supports.)
 Not supported at the moment:

 $ svn mkdir X
 A   X
 $ svn status
 AX
 $ svn diff
 $ svn diff --git
 $

 Suggestion:

 $ svn diff --git
 Index: empty
 ===
 diff --git a/trunk/empty b/trunk/empty
 new directory mode 10644
 IIRC trailing slashes on empty/ were suggested on IRC, what was the
 conclusion about that?

 E.g., just changing the 'new file mode 10644' line to mention directory
 instead. Haven't investigate what changes would be needed in the diff
 editor.
 By the way, are we just influenced by Git's format, or are we looking
 for some degree of interoperability?  Consider adding a symlink:

 [[[
 ### with git (in $wcroot/trunk/):
 diff --git a/trunk/bar b/trunk/bar
 new file mode 12
 index 000..1910281
 --- /dev/null
 +++ b/trunk/bar
 @@ -0,0 +1 @@
 +foo
 \ No newline at end of file

 ### with svn (in $wcroot/trunk/):
 Index: bar
 ===
 diff --git a/trunk/bar b/trunk/bar
 new file mode 10644
 --- /dev/null   (revision 0)
 +++ b/trunk/bar (working copy)
 @@ -0,0 +1 @@
 +link foo
 \ No newline at end of file

 Property changes on: trunk/bar
 ___
 Added: svn:special
 ## -0,0 +1 ##
 +*
 ]]]
Hmm, this is interesting. :) Git faithfully (blindly?) interprets Unix
permission bits, whiles SVN faithfully (blindly?) interprets the
contents of special files ... I wonder if svn patch does the right
thing here?

Anyway, for the sake of interoperability, we'd have to emit and parse
the git format for symlinks. Not that I'm too amused by the idea that
git probably just does a chmod on the new file without thinking about
it, but hey, All the World is Linux, right? :)

-- Brane


Re: Do we want 'svn patch' to be able to add empty files?

2010-09-02 Thread Daniel Näslund
On Thu, Sep 02, 2010 at 11:27:07AM +0300, Daniel Shahaf wrote:
 Daniel Näslund wrote on Thu, Sep 02, 2010 at 07:13:00 +0200:
  On Wed, Sep 01, 2010 at 06:37:08PM +0100, Julian Foad wrote:
   This may be off topic, but I'm wondering whether Git has defined such
   operations on directories fully or at all, since it doesn't version them
   explicitly.  I mean, can you tell the difference between add empty file
   A and add empty dir A?  I could go and look, but haven't time today.
   If yes, great; if it doesn't, we'll have to invent syntax extensions to
   do it.  (I'm recalling that the goal of this work is we want Subversion
   diffs to be able to support all valid Subversion changes, and we chose
   Git format as a basis for supporting that.  We don't want to constrain
   ourselves to only the operations that Git supports.)
  
  Not supported at the moment:
  
  $ svn mkdir X
  A   X
  $ svn status
  AX
  $ svn diff
  $ svn diff --git
  $
  
  Suggestion:
  
  $ svn diff --git
  Index: empty
  ===
  diff --git a/trunk/empty b/trunk/empty
  new directory mode 10644
 
 IIRC trailing slashes on empty/ were suggested on IRC, what was the
 conclusion about that?

I didn't follow that one (atleast I don't recall following it) but will
stay tuned for the resolution of the discussion.

  E.g., just changing the 'new file mode 10644' line to mention directory
  instead. Haven't investigate what changes would be needed in the diff
  editor.
 
 By the way, are we just influenced by Git's format, or are we looking
 for some degree of interoperability?  Consider adding a symlink:

We have two goals I think:
i)  Implement a format that is interoperable with git and mercurial and
possible other vcs tools that choose to implement the git extensions
to the unidiff format.
ii) Adjust the format dictated by the git unidiff extensions to allow us
to describe svn-specific features such as properties, versioned
dirs and copies from a specific revision. Those are the ones that
immediately comes to mind. Please fill in if you have more
suggestions on what the format should include!

As for symlinks, svn has the (in my opinion) ugly solution of storing
the information in an svn:special property. So we have a specific svn
way of specifying symlinks. Originally it was my intention to only use
the svn:special property. For me, the goal of interoperability is clearly not as
important as us having a format that works for svn. If I were to choose
between introducing additional complexity for providing interoperability
or drop that feature (i.e. git symlinks) I would choose the second by
instinct. But maybe the community values i) more than I do?

Note that we don't yet perform any symlink operation when applying a
path with svn:special set.

 [[[
 ### with git (in $wcroot/trunk/):
 diff --git a/trunk/bar b/trunk/bar
 new file mode 12
 index 000..1910281
 --- /dev/null
 +++ b/trunk/bar
 @@ -0,0 +1 @@
 +foo
 \ No newline at end of file
 
 ### with svn (in $wcroot/trunk/):
 Index: bar
 ===
 diff --git a/trunk/bar b/trunk/bar
 new file mode 10644
 --- /dev/null   (revision 0)
 +++ b/trunk/bar (working copy)
 @@ -0,0 +1 @@
 +link foo
 \ No newline at end of file
 
 Property changes on: trunk/bar
 ___
 Added: svn:special
 ## -0,0 +1 ##
 +*
 ]]]

Daniel (Näslund)



Re: Do we want 'svn patch' to be able to add empty files?

2010-09-02 Thread Daniel Näslund
On Thu, Sep 02, 2010 at 11:14:27AM +0300, Daniel Shahaf wrote:
 (Thanks for the examples.  I suppose next time I should try to run
 'touch foo; $svn add foo; $svn diff --git' myself...)
 
 Daniel Näslund wrote on Thu, Sep 02, 2010 at 07:05:41 +0200:
  On Wed, Sep 01, 2010 at 10:54:08PM +0300, Daniel Shahaf wrote:
   Daniel Näslund wrote on Wed, Sep 01, 2010 at 11:28:51 +0200:
(This started out as me trying to apply added paths using the 
information
from a patch file in the git diff format. The only that I could come up
with where an add could not be detected by just looking at the regular
unidiff headers  was adding an empty file (it has no unidiff headers).
If anyone has any other cases, please let me know.)

   
   How does a diff adding an empty file look?
  
  Like this:
  
  Index: empty
  ===
  diff --git a/trunk/empty b/trunk/empty
  new file mode 10644
  
 
 So, it boils down to having to recognize /^new file mode/ (even though
 there are no following /^(---|+++)/ lines), right?
 
 I've never been inside the patch code, I don't know how easy/tricky it
 would be to add this.

See svn_diff_parse_next_patch() for the gory details.

  Note that we allow empty files to be created for regular diffs too if
  they have property changes. This patch will create an empty file with
  property 'foo' set on it:
  
  Index: empty
  ===
  
  Property changes on: empty
  ___
  Added: foo
  ## -0,0 +1 ##
  +value
 
 So this implicitly creates the file if it doesn't exist already; in
 other words, we do not distinguish setting a property on an existing
 file (without content changes) from adding a file with properties.
 Would it be better to make a distinction --- for example, by generating
 a /^new file/ line in the latter case?  (that would be explicit and more
 friendly to non-property-aware tools)

My personal opinion is that it would be better to add a --git option to
svn patch and only apply tree changes and property changes if we have a
git diff and a the --git diff option given. Then we wouln't have these
kind of overlapping cases. But for the user it's more convinient to just
call 'svn patch PATCH_FILE' on whatever file we have at hand.

It's a personal opinion and I'm not really advocating it... Just making
noise I guess. :)

Daniel (Näslund)



Re: Do we want 'svn patch' to be able to add empty files?

2010-09-02 Thread Branko Čibej
 On 02.09.2010 11:10, Daniel Näslund wrote:
 So this implicitly creates the file if it doesn't exist already; in
 other words, we do not distinguish setting a property on an existing
 file (without content changes) from adding a file with properties.
 Would it be better to make a distinction --- for example, by generating
 a /^new file/ line in the latter case?  (that would be explicit and more
 friendly to non-property-aware tools)
 My personal opinion is that it would be better to add a --git option to
 svn patch and only apply tree changes and property changes if we have a
 git diff and a the --git diff option given. Then we wouln't have these
 kind of overlapping cases. But for the user it's more convinient to just
 call 'svn patch PATCH_FILE' on whatever file we have at hand.

 It's a personal opinion and I'm not really advocating it... Just making
 noise I guess. :)

There's no inherent reason (that I can see) why svn patch should need
such an option, is there? Except if we want to introduce a stricter
mode, e.g., that would forbid property changes on nonexistent files
unless the new file tree change is signalled. There's not really any
overlap here.

-- Brane


Re: Do we want 'svn patch' to be able to add empty files?

2010-09-02 Thread Stefan Sperling
On Thu, Sep 02, 2010 at 12:04:15PM +0200, Branko Čibej wrote:
  On 02.09.2010 11:03, Daniel Näslund wrote:
  Note that we don't yet perform any symlink operation when applying a
  path with svn:special set.
 
 If that isn't fixed for 1.7, then it rates a big warning!! in the
 release notes.

Filed as http://subversion.tigris.org/issues/show_bug.cgi?id=3697
with milestone 1.7.0. I think we can fix this before release.

Thanks,
Stefan


Re: Do we want 'svn patch' to be able to add empty files?

2010-09-02 Thread Stefan Sperling
On Thu, Sep 02, 2010 at 11:03:36AM +0200, Daniel Näslund wrote:
 We have two goals I think:
 i)  Implement a format that is interoperable with git and mercurial and
 possible other vcs tools that choose to implement the git extensions
 to the unidiff format.
 ii) Adjust the format dictated by the git unidiff extensions to allow us
 to describe svn-specific features such as properties, versioned
 dirs and copies from a specific revision. Those are the ones that
 immediately comes to mind. Please fill in if you have more
 suggestions on what the format should include!
 
 As for symlinks, svn has the (in my opinion) ugly solution of storing
 the information in an svn:special property. So we have a specific svn
 way of specifying symlinks. Originally it was my intention to only use
 the svn:special property. For me, the goal of interoperability is clearly not 
 as
 important as us having a format that works for svn. If I were to choose
 between introducing additional complexity for providing interoperability
 or drop that feature (i.e. git symlinks) I would choose the second by
 instinct. But maybe the community values i) more than I do?

I'd say that if git already has a way to represent a concept in its
diff format, we should strive to represent the same thing in the same way.

Because the format isn't standardized, we must try to adhere to the
current de-facto standard as much as possible to avoid incompatibilities.
Regardless, we'll probably see compatibility problems sooner or later as
time goes by and each of the git, hg, and svn teams amend the output of
their diff command.  But any known diviation should at be documented and
eventually fixed.

So I think we should aim for eventually representing symlinks just like
git does. svn patch can interpret the special symlink file mode and
create a symlink the svn-way. But this doesn't need to happen for 1.7.
svn patch can easily interpret both the git-way and the svn-way of
representing symlinks. So we can simply change the way svn diff prints
symlinks later, even in 1.8. svn diff could encode the symlink once in
the git diff header and once as a property, to stay backwards compatible.

For now, an svn symlink will show up as an empty file when the patch is
applied with git. I don't think that's a huge problem. Nice to have, but
not release critical for Subversion.

 Note that we don't yet perform any symlink operation when applying a
 path with svn:special set.

We should though. I've filed an issue about that:
http://subversion.tigris.org/issues/show_bug.cgi?id=3697

Stefan


Re: Do we want 'svn patch' to be able to add empty files?

2010-09-02 Thread Stefan Sperling
On Thu, Sep 02, 2010 at 11:27:07AM +0300, Daniel Shahaf wrote:
 Daniel Näslund wrote on Thu, Sep 02, 2010 at 07:13:00 +0200:
  On Wed, Sep 01, 2010 at 06:37:08PM +0100, Julian Foad wrote:
   This may be off topic, but I'm wondering whether Git has defined such
   operations on directories fully or at all, since it doesn't version them
   explicitly.  I mean, can you tell the difference between add empty file
   A and add empty dir A?  I could go and look, but haven't time today.
   If yes, great; if it doesn't, we'll have to invent syntax extensions to
   do it.  (I'm recalling that the goal of this work is we want Subversion
   diffs to be able to support all valid Subversion changes, and we chose
   Git format as a basis for supporting that.  We don't want to constrain
   ourselves to only the operations that Git supports.)

This is a known problem. Git has no way of representing directories
to the best of my knowledge (haven't checked the code...)
I guess we'll need to make up our own way of representing them.

Right now, svn patch will always add an empty file if something is added
that only has props. But the patch might want to create a directory instead.
Is there a 1:1 mapping svn: propery - node kind (file or dir)?
If so, we could get away with just checking the type of property when
deciding whether to create a file or a directory.

Filed as http://subversion.tigris.org/issues/show_bug.cgi?id=3698

 IIRC trailing slashes on empty/ were suggested on IRC, what was the
 conclusion about that?

I don't like this, because it's way too subtle. And it's not clear how other
tools like git and hg would handle this.

I'd prefer adding a new git-diff-style header that says this is a directory,
not a file. If possible, in a way that git and hg would ignore.
 
Stefan


Re: Do we want 'svn patch' to be able to add empty files?

2010-09-02 Thread Branko Čibej
 On 02.09.2010 13:25, Stefan Sperling wrote:
 For now, an svn symlink will show up as an empty file when the patch is
 applied with git. I don't think that's a huge problem. Nice to have, but
 not release critical for Subversion.

That's not how I read the patch ... I believe git will create a file
whose contents are link foo given the example patch in this thread.

-- Brane


Re: Do we want 'svn patch' to be able to add empty files?

2010-09-02 Thread Stefan Sperling
On Thu, Sep 02, 2010 at 02:09:03PM +0200, Branko Čibej wrote:
  On 02.09.2010 13:25, Stefan Sperling wrote:
  For now, an svn symlink will show up as an empty file when the patch is
  applied with git. I don't think that's a huge problem. Nice to have, but
  not release critical for Subversion.
 
 That's not how I read the patch ... I believe git will create a file
 whose contents are link foo given the example patch in this thread.

Yes, you're right. Thanks for correcting.
But still not a release-critical problem.

Stefan


Re: Do we want 'svn patch' to be able to add empty files?

2010-09-02 Thread Julian Foad
On Thu, 2010-09-02 at 14:03 +0200, Stefan Sperling wrote:
 On Thu, Sep 02, 2010 at 01:57:49PM +0200, Stefan Sperling wrote:
  Right now, svn patch will always add an empty file if something is added
  that only has props. But the patch might want to create a directory instead.
  Is there a 1:1 mapping svn: propery - node kind (file or dir)?
  If so, we could get away with just checking the type of property when
  deciding whether to create a file or a directory.
 
 Hmmm... that won't work for custom user properties, so we'll also need
 a more generic way of representing directories in a patch. But this is
 also something we can postpone until after 1.7. We should focus on
 properly supporting the svn: properties first, that's much more important.

Creation of a file or dir should have nothing to do with properties.
Many files and dirs don't even have properties, and for those that do,
we shouldn't care what they are.

I think application of a property diff should require that there is a
node to apply it to, and should not by itself cause the creation (nor
deletion) of a file or dir.  This implies that when svn diff --git
finds a new file with empty content, it should write an add a new file
record before any property-adding diff.  Similarly for dirs.  Similarly
for deleting.

- Julian




Do we want 'svn patch' to be able to add empty files?

2010-09-01 Thread Daniel Näslund
Hi!

(This started out as me trying to apply added paths using the information
from a patch file in the git diff format. The only that I could come up
with where an add could not be detected by just looking at the regular
unidiff headers  was adding an empty file (it has no unidiff headers).
If anyone has any other cases, please let me know.)

The question is: Do we want 'svn patch to be able to add empty files.

+ It's consistent with the idea of a git diff dealing with tree changes.
- It adds an extra special case to the code. I've seen GNU patch with
  its gigantic if-spaghetti's. Just don't want to obscure the code with
  something that's not needed.
- There might not be a use case for it, though I think I've heard of the
  use of empty files as markers. Just can't come up with an example
  right now.
  
Daniel


Re: Do we want 'svn patch' to be able to add empty files?

2010-09-01 Thread Julian Foad
Daniel Näslund wrote:
 Hi!
 
 (This started out as me trying to apply added paths using the information
 from a patch file in the git diff format. The only that I could come up
 with where an add could not be detected by just looking at the regular
 unidiff headers  was adding an empty file (it has no unidiff headers).
 If anyone has any other cases, please let me know.)
 
 The question is: Do we want 'svn patch to be able to add empty files.
 
 + It's consistent with the idea of a git diff dealing with tree changes.
 - It adds an extra special case to the code. I've seen GNU patch with
   its gigantic if-spaghetti's. Just don't want to obscure the code with
   something that's not needed.
 - There might not be a use case for it, though I think I've heard of the
   use of empty files as markers. Just can't come up with an example
   right now.

If add an empty file is a well defined operation in Git-diff syntax,
then yes, it would be good to support it.  As well as delete this empty
file and any other valid Subversion operations that aren't yet
supported.

This may be off topic, but I'm wondering whether Git has defined such
operations on directories fully or at all, since it doesn't version them
explicitly.  I mean, can you tell the difference between add empty file
A and add empty dir A?  I could go and look, but haven't time today.
If yes, great; if it doesn't, we'll have to invent syntax extensions to
do it.  (I'm recalling that the goal of this work is we want Subversion
diffs to be able to support all valid Subversion changes, and we chose
Git format as a basis for supporting that.  We don't want to constrain
ourselves to only the operations that Git supports.)

- Julian




Re: Do we want 'svn patch' to be able to add empty files?

2010-09-01 Thread Daniel Shahaf
Daniel Näslund wrote on Wed, Sep 01, 2010 at 11:28:51 +0200:
 (This started out as me trying to apply added paths using the information
 from a patch file in the git diff format. The only that I could come up
 with where an add could not be detected by just looking at the regular
 unidiff headers  was adding an empty file (it has no unidiff headers).
 If anyone has any other cases, please let me know.)
 

How does a diff adding an empty file look?

 The question is: Do we want 'svn patch to be able to add empty files.
 
 + It's consistent with the idea of a git diff dealing with tree changes.
 - It adds an extra special case to the code. I've seen GNU patch with
   its gigantic if-spaghetti's. Just don't want to obscure the code with
   something that's not needed.

 - There might not be a use case for it, though I think I've heard of the
   use of empty files as markers. Just can't come up with an example
   right now.

Two examples:

* vimrc. (Vim behaves differently with no vimrc than with an empty vimrc)

* pseudo-targets for makefiles
foo:
$commands
touch $@




Re: Do we want 'svn patch' to be able to add empty files?

2010-09-01 Thread Daniel Näslund
On Wed, Sep 01, 2010 at 10:54:08PM +0300, Daniel Shahaf wrote:
 Daniel Näslund wrote on Wed, Sep 01, 2010 at 11:28:51 +0200:
  (This started out as me trying to apply added paths using the information
  from a patch file in the git diff format. The only that I could come up
  with where an add could not be detected by just looking at the regular
  unidiff headers  was adding an empty file (it has no unidiff headers).
  If anyone has any other cases, please let me know.)
  
 
 How does a diff adding an empty file look?

Like this:

Index: empty
===
diff --git a/trunk/empty b/trunk/empty
new file mode 10644

Note that we allow empty files to be created for regular diffs too if
they have property changes. This patch will create an empty file with
property 'foo' set on it:

Index: empty
===

Property changes on: empty
___
Added: foo
## -0,0 +1 ##
+value

  The question is: Do we want 'svn patch to be able to add empty files.
  
  + It's consistent with the idea of a git diff dealing with tree changes.
  - It adds an extra special case to the code. I've seen GNU patch with
its gigantic if-spaghetti's. Just don't want to obscure the code with
something that's not needed.
 
  - There might not be a use case for it, though I think I've heard of the
use of empty files as markers. Just can't come up with an example
right now.
 
 Two examples:
 
 * vimrc. (Vim behaves differently with no vimrc than with an empty vimrc)
 
 * pseudo-targets for makefiles
 foo:
   $commands
   touch $@

Good examples. I think configuration files and build files are the
typical use cases for empty files.

Daniel (Näslund)