You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Peter N. Lundblad" <pe...@famlundblad.se> on 2005/11/05 12:43:51 UTC

Re: svn commit: r17181 - trunk/subversion/libsvn_wc

On Thu, 3 Nov 2005 dionisos@tigris.org wrote:

> Author: dionisos
> Date: Thu Nov  3 16:37:13 2005
> New Revision: 17181
>
> Modified:
>    trunk/subversion/libsvn_wc/adm_files.c
>    trunk/subversion/libsvn_wc/adm_files.h
>    trunk/subversion/libsvn_wc/diff.c
>    trunk/subversion/libsvn_wc/wc.h
>
> Log:
> Remove creation and use of 'empty-file' from the .svn/ area.
>
> Note:
>   Normally this would require a wc-format bump, but since we already have
>   a post-1.3 format bump, there is currently no need.
>
> * subversion/libsvn_wc/wc.h: Remove the SVN_WC__ADM_EMPTY_FILE define.
>
> * subversion/libsvn_wc/adm_files.h
> * subversion/libsvn_wc/adm_files.c
>   Remove the empty file API and make sure 'empty-file' isn't created
>   in new working copies anymore.
>
> * subversion/libsvn_wc/diff.c (edit_baton): Add an 'empty_file' field
>   for caching the name of the generated empty file.

I've always wondered why we don't use the platforms null device for this.
We have SVN_NULL_DEVICE_NAME which we use to provide an stdin file for
hook scripts already. May it cause problems for external diffs when it is
not a regular file?

Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r17181 - trunk/subversion/libsvn_wc

Posted by John Szakmeister <jo...@szakmeister.net>.
On Saturday 05 November 2005 09:27, Branko Čibej wrote:
[snip]
>
> We've been through this before; search the archives. The short answer
> is: NUL: on Windows is equivalent to /dev/null on Unix; both are bit
> buckets for writing and empty files for reading.
>
> What I can't find in the archives, and can't remember a reason for, is
> why we used .svn/empty-file despite having earlier confirmed the above.

FWIW, I created a patch for this (never sent it to the list though).  I tried 
in both on Linux and Windows and it worked very well, with one exception.  I 
configured Subversion to use an external diff utility (the one from unxutils 
at SourceForge), and it had issues with being passed NUL:.  For whatever 
reason, it seems like it can't handle it. :-/  *shrug*  Perhaps, I did 
something wrong?  At any rate, it was a very easy change to make, and no 
tests broke as a result of the change.

-John

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn commit: r17181 - trunk/subversion/libsvn_wc

Posted by Erik Huelsmann <eh...@gmail.com>.
On 11/5/05, Branko Čibej <br...@xbc.nu> wrote:
> David James wrote:
> > On 11/5/05, Erik Huelsmann <eh...@gmail.com> wrote:
> >
> >>> (If someone on Windows uses a diff tool that can't handle a NUL: as a
> >>> file name, they can write a wrapper for it. I don't think we should
> >>> cater to every half-ported utility in the world.)
> >>>
> >> Great! Well, I was already planning to use SVN_NULL_DEVICE_NAME (but
> >> was looking in the wrong place: APR). Glad we have even discussed it
> >> now and having been pointed in the right direction.
> >>
> >> Consider it implemented!
> >>
> > Is it possible to implement this behaviour without breaking external diff tools?
> >
> As I said, any properly implemented external diff tool should do the
> right thing with "NULL:" -- if it doesn't, it's broken already. We could
> create an empty file before calling an external tool with a "NULL:" file
> name argument and replacing the name, but I don't think we should bother.
>
> A Windows app that doesn't handle " NUL:" correctly is as broken as one
> that can't tell the difference between "C:foo.txt" and "C:\foo.txt". I
> suspect this will only happen in tools that have been ported to Windows
> from (usually) Unix -- and whoever ported it can fix the bug. Not to
> mention that there's a really trivial workaround.

Turns out we weren't exactly without problems either. I'll revert the
commit and document in the code why that was necessary.

We've already reduced the number of temp files from 1 per directory to
1 per diff/commit.

bye,

Erik.

Re: svn commit: r17181 - trunk/subversion/libsvn_wc

Posted by Branko Čibej <br...@xbc.nu>.
David James wrote:
> On 11/5/05, Erik Huelsmann <eh...@gmail.com> wrote:
>   
>>> (If someone on Windows uses a diff tool that can't handle a NUL: as a
>>> file name, they can write a wrapper for it. I don't think we should
>>> cater to every half-ported utility in the world.)
>>>       
>> Great! Well, I was already planning to use SVN_NULL_DEVICE_NAME (but
>> was looking in the wrong place: APR). Glad we have even discussed it
>> now and having been pointed in the right direction.
>>
>> Consider it implemented!
>>     
> Is it possible to implement this behaviour without breaking external diff tools?
>   
As I said, any properly implemented external diff tool should do the 
right thing with "NULL:" -- if it doesn't, it's broken already. We could 
create an empty file before calling an external tool with a "NULL:" file 
name argument and replacing the name, but I don't think we should bother.

A Windows app that doesn't handle " NUL:" correctly is as broken as one 
that can't tell the difference between "C:foo.txt" and "C:\foo.txt". I 
suspect this will only happen in tools that have been ported to Windows 
from (usually) Unix -- and whoever ported it can fix the bug. Not to 
mention that there's a really trivial workaround.

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r17181 - trunk/subversion/libsvn_wc

Posted by David James <ja...@gmail.com>.
On 11/5/05, Erik Huelsmann <eh...@gmail.com> wrote:
> > (If someone on Windows uses a diff tool that can't handle a NUL: as a
> > file name, they can write a wrapper for it. I don't think we should
> > cater to every half-ported utility in the world.)
>
> Great! Well, I was already planning to use SVN_NULL_DEVICE_NAME (but
> was looking in the wrong place: APR). Glad we have even discussed it
> now and having been pointed in the right direction.
>
> Consider it implemented!
Is it possible to implement this behaviour without breaking external diff tools?

Cheers,

David



--
David James -- http://www.cs.toronto.edu/~james

Re: svn commit: r17181 - trunk/subversion/libsvn_wc

Posted by Erik Huelsmann <eh...@gmail.com>.
> (If someone on Windows uses a diff tool that can't handle a NUL: as a
> file name, they can write a wrapper for it. I don't think we should
> cater to every half-ported utility in the world.)

Great! Well, I was already planning to use SVN_NULL_DEVICE_NAME (but
was looking in the wrong place: APR). Glad we have even discussed it
now and having been pointed in the right direction.

Consider it implemented!

bye,

Erik.

Re: Diff: null device instead of empty file? [was: svn commit: r17181 - trunk/subversion/libsvn_wc]

Posted by Julian Foad <ju...@btopenworld.com>.
Branko Čibej wrote:
> Julian Foad wrote:
>>
>> To clarify my position: I have done some general programming under 
>> DOS/Windows, including stdio, in which I know it is easy to open and 
>> read "NUL:", but I have less experience of the file-handle APIs that 
>> are (AFAIK) much more widely used, and cannot definitively answer this 
>> question.  From what little knowledge and experience I have both as a 
>> programmer and as a user, my impression is that devices (including 
>> NUL) would often not be supported.
>>
>> I don't know what Branko's position of authority is on the matter.  I 
>> reacted based on the impression that he was as unqualified as I am; 
>> apologies if this is not so.
> 
> Heh, looks like you got some private mail from somewhere. :)

No.  I just realised after sending my previous message that it wasn't 
particularly clever of me to just state a contrary opinion without indicating 
why I thought that way.  Then I remembered that you currently do some Windows 
programming so were more likely to be speaking from knowledge than I was...

> The special device names are supported by the Win32 (and now Win64...) 
> API layer directly. As I said, a Windows app would have to go out of its 
> way to make them _not_ work. Interestingly enough, Word /is/ such [...]

Oh, OK.  I'm glad to hear it.  (I wrote the message quoted above before you 
first said that.)

> (BTW, since /dev/null is actually a character device, not a regular 
> file, I suppose it's about as probable that a Unix diff tool refuses to 
> work with /dev/null as it is that a Windows app chokes on NUL:...).

OK!

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Diff: null device instead of empty file? [was: svn commit: r17181 - trunk/subversion/libsvn_wc]

Posted by Branko Čibej <br...@xbc.nu>.
Julian Foad wrote:
> Julian Foad wrote:
>> Branko Čibej wrote:
>>
>>> Hm, yes, an external diff could have problems. I'd also call it 
>>> broken, though -- Cygwin's GNU diff can handle NULL. Unix programs 
>>> should have no problems at all, because for all they care, /dev/null 
>>> is just another file.
> [...]
>>
>> I don't accept that diff programs should be expected to handle NUL:.  
> [...]
>
> To clarify my position: I have done some general programming under 
> DOS/Windows, including stdio, in which I know it is easy to open and 
> read "NUL:", but I have less experience of the file-handle APIs that 
> are (AFAIK) much more widely used, and cannot definitively answer this 
> question.  From what little knowledge and experience I have both as a 
> programmer and as a user, my impression is that devices (including 
> NUL) would often not be supported.
>
> I don't know what Branko's position of authority is on the matter.  I 
> reacted based on the impression that he was as unqualified as I am; 
> apologies if this is not so.
Heh, looks like you got some private mail from somewhere. :)

The special device names are supported by the Win32 (and now Win64...) 
API layer directly. As I said, a Windows app would have to go out of its 
way to make them _not_ work. Interestingly enough, Word /is/ such an 
app, but then it does file locking and all sorts of other "interesting" 
things with the file name. I wouldn't expect any reasonable diff/merge 
tool to do that, so I'm not worried.

(BTW, since /dev/null is actually a character device, not a regular 
file, I suppose it's about as probable that a Unix diff tool refuses to 
work with /dev/null as it is that a Windows app chokes on NUL:...).

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Diff: null device instead of empty file? [was: svn commit: r17181 - trunk/subversion/libsvn_wc]

Posted by Julian Foad <ju...@btopenworld.com>.
Julian Foad wrote:
> Branko Čibej wrote:
> 
>> Hm, yes, an external diff could have problems. I'd also call it 
>> broken, though -- Cygwin's GNU diff can handle NULL. Unix programs 
>> should have no problems at all, because for all they care, /dev/null 
>> is just another file.
[...]
> 
> I don't accept that diff programs should be expected to handle NUL:.  
[...]

To clarify my position: I have done some general programming under DOS/Windows, 
including stdio, in which I know it is easy to open and read "NUL:", but I have 
less experience of the file-handle APIs that are (AFAIK) much more widely used, 
and cannot definitively answer this question.  From what little knowledge and 
experience I have both as a programmer and as a user, my impression is that 
devices (including NUL) would often not be supported.

I don't know what Branko's position of authority is on the matter.  I reacted 
based on the impression that he was as unqualified as I am; apologies if this 
is not so.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Diff: null device instead of empty file? [was: svn commit: r17181 - trunk/subversion/libsvn_wc]

Posted by Michael Sinz <Mi...@sinz.org>.
On 11/6/05, Branko Čibej <br...@xbc.nu> wrote:
> Julian Foad wrote:
[...]
> > to do so?  And that's not a totally unfair comparison.  Diff programs
> > are not all line-based stdin-reading text stream consumers.
> Any diff program we're able to call from SVN will know how to open a
> file. There's nothing special about opening NUL: on Windows; you simply
> pass "NUL:" to CreateFile or fopen or open or whatever, and it should
> Just Work. You have to make a special effort for it not to.

Well, maybe, but maybe not - one thing that I have done in the past
is to use mmap() like symantics to read large files.  It makes the diff
operation just operations on a memory array and lets the VM system
do the optimal reading/paging/caching of the data.

Now, I don't know how/if the windows mmap() like behaviors work on
things like null: (/dev/null) but I could see this as a potential source of
problems.

(Not that I don't think that programs are broken if they can not read
or write to /dev/null, but sometimes they just are...)

--
Michael Sinz               Technology and Engineering Director/Consultant
"Starting Startups"                          mailto:Michael.Sinz@sinz.org
My place on the web                      http://www.sinz.org/Michael.Sinz

Re: Diff: null device instead of empty file? [was: svn commit: r17181 - trunk/subversion/libsvn_wc]

Posted by Branko Čibej <br...@xbc.nu>.
Julian Foad wrote:
> Branko Čibej wrote:
>> Erik Huelsmann wrote:
>>>
>>> I'd like to just call external diffs not working with NUL: broken,
>>> though, but should we try to move this change into 1.4RC1 and just
>>> see? I'm all for it.
>>>
>> Hm, yes, an external diff could have problems. I'd also call it 
>> broken, though -- Cygwin's GNU diff can handle NULL. Unix programs 
>> should have no problems at all, because for all they care, /dev/null 
>> is just another file.
>>
>> (If someone on Windows uses a diff tool that can't handle a NUL: as a 
>> file name, they can write a wrapper for it. I don't think we should 
>> cater to every half-ported utility in the world.)
>
> I don't accept that diff programs should be expected to handle NUL:.  
> The distinction between devices and files is greater on MS DOS/Windows 
> than on Unix-like OSes, and it would be reasonable for a diff program 
> to require regular files.
>
>
>   $ vim /dev/null
>
> works fine, but would you expect
>
>   C:\> msword NUL:
>
> to do so?  And that's not a totally unfair comparison.  Diff programs 
> are not all line-based stdin-reading text stream consumers.
Any diff program we're able to call from SVN will know how to open a 
file. There's nothing special about opening NUL: on Windows; you simply 
pass "NUL:" to CreateFile or fopen or open or whatever, and it should 
Just Work. You have to make a special effort for it not to.

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Diff: null device instead of empty file? [was: svn commit: r17181 - trunk/subversion/libsvn_wc]

Posted by Julian Foad <ju...@btopenworld.com>.
Branko Čibej wrote:
> Erik Huelsmann wrote:
>>
>> I'd like to just call external diffs not working with NUL: broken,
>> though, but should we try to move this change into 1.4RC1 and just
>> see? I'm all for it.
>>
> Hm, yes, an external diff could have problems. I'd also call it broken, 
> though -- Cygwin's GNU diff can handle NULL. Unix programs should have 
> no problems at all, because for all they care, /dev/null is just another 
> file.
> 
> (If someone on Windows uses a diff tool that can't handle a NUL: as a 
> file name, they can write a wrapper for it. I don't think we should 
> cater to every half-ported utility in the world.)

I don't accept that diff programs should be expected to handle NUL:.  The 
distinction between devices and files is greater on MS DOS/Windows than on 
Unix-like OSes, and it would be reasonable for a diff program to require 
regular files.


   $ vim /dev/null

works fine, but would you expect

   C:\> msword NUL:

to do so?  And that's not a totally unfair comparison.  Diff programs are not 
all line-based stdin-reading text stream consumers.


I can join you in wishing that all diff programs would accept NUL:, but I don't 
for a moment expect they will.

So that leaves us with just the "users can work around it" argument.  No 
comment on that at present.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r17181 - trunk/subversion/libsvn_wc

Posted by Branko Čibej <br...@xbc.nu>.
Erik Huelsmann wrote:

>>>>I've always wondered why we don't use the platforms null device for this.
>>>>We have SVN_NULL_DEVICE_NAME which we use to provide an stdin file for
>>>>hook scripts already. May it cause problems for external diffs when it is
>>>>not a regular file?
>>>>
>>>>        
>>>>
>>>I have *no* idea. I thought I had seen something like a null device
>>>define, but went looking for it in APR (why isn't it in APR?!)
>>>
>>>We'd have to try. On unix, I can't imagine it not working, but have no
>>>idea on other platforms... (I'd sure like to try though!)
>>>
>>>      
>>>
>>We've been through this before; search the archives. The short answer
>>is: NUL: on Windows is equivalent to /dev/null on Unix; both are bit
>>buckets for writing and empty files for reading.
>>
>>What I can't find in the archives, and can't remember a reason for, is
>>why we used .svn/empty-file despite having earlier confirmed the above.
>>    
>>
>
>I wasn't referring to using NUL: for the empty file internally. I
>don't expect problems there, but should we expect external diff
>programs to work with it?
>
>I'd like to just call external diffs not working with NUL: broken,
>though, but should we try to move this change into 1.4RC1 and just
>see? I'm all for it.
>  
>
Hm, yes, an external diff could have problems. I'd also call it broken, 
though -- Cygwin's GNU diff can handle NULL. Unix programs should have 
no problems at all, because for all they care, /dev/null is just another 
file.

(If someone on Windows uses a diff tool that can't handle a NUL: as a 
file name, they can write a wrapper for it. I don't think we should 
cater to every half-ported utility in the world.)

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r17181 - trunk/subversion/libsvn_wc

Posted by Erik Huelsmann <eh...@gmail.com>.
> >> I've always wondered why we don't use the platforms null device for this.
> >> We have SVN_NULL_DEVICE_NAME which we use to provide an stdin file for
> >> hook scripts already. May it cause problems for external diffs when it is
> >> not a regular file?
> >>
> >
> > I have *no* idea. I thought I had seen something like a null device
> > define, but went looking for it in APR (why isn't it in APR?!)
> >
> > We'd have to try. On unix, I can't imagine it not working, but have no
> > idea on other platforms... (I'd sure like to try though!)
> >
> We've been through this before; search the archives. The short answer
> is: NUL: on Windows is equivalent to /dev/null on Unix; both are bit
> buckets for writing and empty files for reading.
>
> What I can't find in the archives, and can't remember a reason for, is
> why we used .svn/empty-file despite having earlier confirmed the above.

I wasn't referring to using NUL: for the empty file internally. I
don't expect problems there, but should we expect external diff
programs to work with it?

I'd like to just call external diffs not working with NUL: broken,
though, but should we try to move this change into 1.4RC1 and just
see? I'm all for it.

bye,

Erik.

Re: svn commit: r17181 - trunk/subversion/libsvn_wc

Posted by Philip Martin <ph...@codematters.co.uk>.
Branko Čibej <br...@xbc.nu> writes:

> We've been through this before; search the archives. The short answer
> is: NUL: on Windows is equivalent to /dev/null on Unix; both are bit
> buckets for writing and empty files for reading.
>
> What I can't find in the archives, and can't remember a reason for, is
> why we used .svn/empty-file despite having earlier confirmed the above.

I think .svn/empty-file appeared before we had configure support for
the null device.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn commit: r17181 - trunk/subversion/libsvn_wc

Posted by Branko Čibej <br...@xbc.nu>.
Erik Huelsmann wrote:
> On 11/5/05, Peter N. Lundblad <pe...@famlundblad.se> wrote:
>   
>> On Thu, 3 Nov 2005 dionisos@tigris.org wrote:
>>
>>     
>>> Author: dionisos
>>> Date: Thu Nov  3 16:37:13 2005
>>> New Revision: 17181
>>>
>>> Modified:
>>>    trunk/subversion/libsvn_wc/adm_files.c
>>>    trunk/subversion/libsvn_wc/adm_files.h
>>>    trunk/subversion/libsvn_wc/diff.c
>>>    trunk/subversion/libsvn_wc/wc.h
>>>
>>> Log:
>>> Remove creation and use of 'empty-file' from the .svn/ area.
>>>
>>> Note:
>>>   Normally this would require a wc-format bump, but since we already have
>>>   a post-1.3 format bump, there is currently no need.
>>>
>>> * subversion/libsvn_wc/wc.h: Remove the SVN_WC__ADM_EMPTY_FILE define.
>>>
>>> * subversion/libsvn_wc/adm_files.h
>>> * subversion/libsvn_wc/adm_files.c
>>>   Remove the empty file API and make sure 'empty-file' isn't created
>>>   in new working copies anymore.
>>>
>>> * subversion/libsvn_wc/diff.c (edit_baton): Add an 'empty_file' field
>>>   for caching the name of the generated empty file.
>>>       
>> I've always wondered why we don't use the platforms null device for this.
>> We have SVN_NULL_DEVICE_NAME which we use to provide an stdin file for
>> hook scripts already. May it cause problems for external diffs when it is
>> not a regular file?
>>     
>
> I have *no* idea. I thought I had seen something like a null device
> define, but went looking for it in APR (why isn't it in APR?!)
>
> We'd have to try. On unix, I can't imagine it not working, but have no
> idea on other platforms... (I'd sure like to try though!)
>   
We've been through this before; search the archives. The short answer 
is: NUL: on Windows is equivalent to /dev/null on Unix; both are bit 
buckets for writing and empty files for reading.

What I can't find in the archives, and can't remember a reason for, is 
why we used .svn/empty-file despite having earlier confirmed the above.

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r17181 - trunk/subversion/libsvn_wc

Posted by Erik Huelsmann <eh...@gmail.com>.
On 11/5/05, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> On Thu, 3 Nov 2005 dionisos@tigris.org wrote:
>
> > Author: dionisos
> > Date: Thu Nov  3 16:37:13 2005
> > New Revision: 17181
> >
> > Modified:
> >    trunk/subversion/libsvn_wc/adm_files.c
> >    trunk/subversion/libsvn_wc/adm_files.h
> >    trunk/subversion/libsvn_wc/diff.c
> >    trunk/subversion/libsvn_wc/wc.h
> >
> > Log:
> > Remove creation and use of 'empty-file' from the .svn/ area.
> >
> > Note:
> >   Normally this would require a wc-format bump, but since we already have
> >   a post-1.3 format bump, there is currently no need.
> >
> > * subversion/libsvn_wc/wc.h: Remove the SVN_WC__ADM_EMPTY_FILE define.
> >
> > * subversion/libsvn_wc/adm_files.h
> > * subversion/libsvn_wc/adm_files.c
> >   Remove the empty file API and make sure 'empty-file' isn't created
> >   in new working copies anymore.
> >
> > * subversion/libsvn_wc/diff.c (edit_baton): Add an 'empty_file' field
> >   for caching the name of the generated empty file.
>
> I've always wondered why we don't use the platforms null device for this.
> We have SVN_NULL_DEVICE_NAME which we use to provide an stdin file for
> hook scripts already. May it cause problems for external diffs when it is
> not a regular file?

I have *no* idea. I thought I had seen something like a null device
define, but went looking for it in APR (why isn't it in APR?!)

We'd have to try. On unix, I can't imagine it not working, but have no
idea on other platforms... (I'd sure like to try though!)

bye,


Erik.