Re: Do we want 'svn patch' to be able to add empty files?
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?
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?
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?
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?
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?
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?
(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?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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)