Re: filename ends with a space, cannot use the --targets option.
Branko Čibej <[email protected]> Tue, 17 Jun 2025 15:57:21 +0200
| Newsgroups | gmane.comp.version-control.subversion.user |
|---|---|
| Organization | The Apache Software Foundation |
| Message-ID | <[email protected]> |
This is a multi-part message in MIME format. --------------GG0wqVioI0vb8A0dIUqcq7Rj Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 17. 6. 25 09:45, Daniel Sahlberg wrote: > Den tis 17 juni 2025 kl 06:24 skrev Yasuhito FUTATSUKI > <[email protected]>: > > Hello, > > On 2025/06/16 18:06, 管理员 wrote: > > If the file name ends with a space, it cannot be save to a file > then use the --targets options: > > > > For example: > > > > Files: > > > > /Users/abc/trunk/aaa > > /Users/abc/trunk/aa // ends with a space. > > > > create a file: /Users/abc/trunk/1.txt, contents: > > > > /Users/abc/trunk/aaa > > /Users/abc/trunk/aa // ends with a space. > > > > execute svn command: > > > > svn add --targets /Users/abc/trunk/1.txt > > > > svn: warning: W155010: '/Users/abc/trunk/aa' not found > > svn: E200009: Could not add all targets because some targets > don't exist > > svn: E200009: Illegal target for the requested operation > > > > Subversion client version: 1.14.5 > > Subversion server version: 1.14.5 > > OS: macOS 15.5 > > I could confirm the issue. With --targets option, the content > of the target file is splited by new line, and each lines are > trimed trailing white space(s). > > The behaviour can be changed with following patch, but I have no > confidence that the current behaviour is intended or not. > [[[ > Index: subversion/svn/svn.c > =================================================================== > --- subversion/svn/svn.c (revision 1926152) > +++ subversion/svn/svn.c (working copy) > @@ -2409,7 +2409,7 @@ > SVN_ERR(svn_stringbuf_from_file2(&buffer, utf8_opt_arg, pool)); > SVN_ERR(svn_utf_stringbuf_to_utf8(&buffer_utf8, buffer, pool)); > opt_state.targets = > svn_cstring_split(buffer_utf8->data, "\n\r", > - TRUE, pool); > + FALSE, pool); > } > break; > case opt_force: > ]]] > > Note: Of course, file names endts with spaces can be add > by passing exact the file name to svn add, e.g. with most > shells on Unix like OS, > > $ svn add '/Users/abc/trunk/aa ' > > or > > $ svn add /Users/abc/trunk/aa\ > ~ one space here. > Cheers, > -- > Yasuhito FUTATSUKI <[email protected]> > > > Hi, > > Best as I can tell this was changed in r842098 > <https://svn.apache.org/r842098> where the original call to > svn_cl__newline_to_array() was replaced with svn_cstring_split() with > the TRUE argument: > [[[ > @@ -950,7 +949,8 @@ > svn_pool_destroy (pool); > return EXIT_FAILURE; > } > - opt_state.targets = svn_cl__newlinelist_to_array(buffer, pool); > + opt_state.targets = svn_cstring_split (buffer->data, "\n\r", > + TRUE, pool); > ]]] > > If I'm reading the code right (the function > svn_cl__newlinelist_to_array() was removed in 842098 so you need to do > `svn cat subversion/clients/cmdline/util.c@842097`), LEADING > whitespace was removed/ignored but trailing whitespace was kept > intact. The change above would resolve the reported issue but also > keeping leading whitespace. > > For the record, r842098 was committed in 2002! > > So I see a few different possible actions > 1. Keep current behaviour (basically declaring this works as > expected). This has been the behaviour since at least Subversion 1.1. > 2. Commit the patch above, changing behaviour for those relying on > trimming preceding (and possibly trailing) whitespace. > 3. Recreating behaviour of svn_cl__newlinelist_to_array() by adding a > separate parameter to svn_cstring_split() so we can control separately > the removal of preceding and trailing whitespace. (This is public API > so we need to create a new svn_cstring_split2()). > > I personally think it should be possible to specify any legal > filenames in the --targets file. Files with preceding or trailing > whitespace is possible under Linux. It seems NTFS also allow this but > the Windows shell trim both ends, cmd.exe allow creating files with > preceding whitespace. I haven't checked macOS. > > I'm less inclined to restore the original behaviour (ie option 3) > since it files with preceding whitespace would still not be possible > to specify in the --targets file AND we would create regressions for > people who expect trailing whitespace to be trimmed. > > Thus I'm leaning towards 2, even though it may create a regression for > any user of Subversion 1.1 or later. Let's call it +0.8. Before you decide to "let's do it because it's possible", think about what such file names are good for. Other than confusing people, that is, or even creating some malicious repository. Consider the consequences: * Users won't see the trailing spaces in any file manager or shell. * You can't manipulate such files at all, using the Windows command line. * Such a change would not be backward compatible. While you can do pretty much anything in Unix shells, the question is -- should you? I think this "use case" falls in the same category as committing files with \ or ^H (that's backspace) in their names. Here's an example: brane@zulu:~/src/bs$ /bin/ls -wl total 0 -rw-r--r-- 1 brane staff 0 Jun 17 15:49 foo.c -rw-r--r-- 1 brane staff 0 Jun 17 15:48 foo.c -rw-r--r-- 1 brane staff 0 Jun 17 15:49 foo.c All three file names are unique. Which of them have trailing spaces? Hint: only one ... but hey, that's neat! We can rename all of Subversion's files and directories to look like 'foo' or 'foo.c', wouldn't that be nice? TL;DR: Leave well enough alone. One can already do any amount of mischief with a Unix shell, or by creating files programmatically. There's really no need to add one more avenue for confusion by "fixing" --targets. -- Brane --------------GG0wqVioI0vb8A0dIUqcq7Rj Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit <!DOCTYPE html> <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body> <div class="moz-cite-prefix">On 17. 6. 25 09:45, Daniel Sahlberg wrote:<br> </div> <blockquote type="cite" cite="mid:CAMHy98OxB5ek9GsOcV7QXNUi=CAgQymD8d1MfAtTprtJcvX4wA@mail.gmail.com"> <meta http-equiv="content-type" content="text/html; charset=UTF-8"> <div dir="ltr"> <div dir="ltr"> <div dir="ltr"> <div dir="ltr"> <div dir="ltr"> <div dir="ltr"> <div dir="ltr"> <div dir="ltr">Den tis 17 juni 2025 kl 06:24 skrev Yasuhito FUTATSUKI <<a href="mailto:[email protected]" moz-do-not-send="true" class="moz-txt-link-freetext">[email protected]</a>>:</div> <div class="gmail_quote gmail_quote_container"> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello,<br> <br> On 2025/06/16 18:06, 管理员 wrote:<br> > If the file name ends with a space, it cannot be save to a file then use the --targets options:<br> > <br> > For example:<br> > <br> > Files:<br> > <br> > /Users/abc/trunk/aaa<br> > /Users/abc/trunk/aa // ends with a space.<br> > <br> > create a file: /Users/abc/trunk/1.txt, contents:<br> > <br> > /Users/abc/trunk/aaa<br> > /Users/abc/trunk/aa // ends with a space.<br> > <br> > execute svn command:<br> > <br> > svn add --targets /Users/abc/trunk/1.txt<br> > <br> > svn: warning: W155010: '/Users/abc/trunk/aa' not found<br> > svn: E200009: Could not add all targets because some targets don't exist<br> > svn: E200009: Illegal target for the requested operation<br> <br> <br> > Subversion client version: 1.14.5<br> > Subversion server version: 1.14.5<br> > OS: macOS 15.5<br> <br> I could confirm the issue. With --targets option, the content<br> of the target file is splited by new line, and each lines are<br> trimed trailing white space(s).<br> <br> The behaviour can be changed with following patch, but I have no<br> confidence that the current behaviour is intended or not.<br> [[[<br> Index: subversion/svn/svn.c<br> ===================================================================<br> --- subversion/svn/svn.c (revision 1926152)<br> +++ subversion/svn/svn.c (working copy)<br> @@ -2409,7 +2409,7 @@<br> SVN_ERR(svn_stringbuf_from_file2(&buffer, utf8_opt_arg, pool));<br> SVN_ERR(svn_utf_stringbuf_to_utf8(&buffer_utf8, buffer, pool));<br> opt_state.targets = svn_cstring_split(buffer_utf8->data, "\n\r",<br> - TRUE, pool);<br> + FALSE, pool);<br> }<br> break;<br> case opt_force:<br> ]]]<br> <br> Note: Of course, file names endts with spaces can be add<br> by passing exact the file name to svn add, e.g. with most<br> shells on Unix like OS,<br> <br> $ svn add '/Users/abc/trunk/aa '<br> <br> or<br> <br> $ svn add /Users/abc/trunk/aa\<br> ~ one space here.<br> Cheers,<br> -- <br> Yasuhito FUTATSUKI <<a href="mailto:[email protected]" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">[email protected]</a>><br> <br> </blockquote> <div><br> </div> <div>Hi,</div> <div><br> </div> <div>Best as I can tell this was changed in r<a href="https://svn.apache.org/r842098" moz-do-not-send="true">842098</a> where the original call to svn_cl__newline_to_array() was replaced with svn_cstring_split() with the TRUE argument:</div> <div>[[[</div> <div> <div>@@ -950,7 +949,8 @@</div> <div> svn_pool_destroy (pool);</div> <div> return EXIT_FAILURE;</div> <div> }</div> <div>- opt_state.targets = svn_cl__newlinelist_to_array(buffer, pool);</div> <div>+ opt_state.targets = svn_cstring_split (buffer->data, "\n\r",</div> <div>+ TRUE, pool);</div> </div> <div>]]]</div> <div><br> </div> <div>If I'm reading the code right (the function svn_cl__newlinelist_to_array() was removed in 842098 so you need to do `svn cat subversion/clients/cmdline/util.c@842097`), LEADING whitespace was removed/ignored but trailing whitespace was kept intact. The change above would resolve the reported issue but also keeping leading whitespace.</div> <div><br> </div> <div>For the record, r842098 was committed in 2002!</div> <div><br> </div> <div>So I see a few different possible actions</div> <div>1. Keep current behaviour (basically declaring this works as expected). This has been the behaviour since at least Subversion 1.1.</div> <div>2. Commit the patch above, changing behaviour for those relying on trimming preceding (and possibly trailing) whitespace.</div> <div>3. Recreating behaviour of svn_cl__newlinelist_to_array() by adding a separate parameter to svn_cstring_split() so we can control separately the removal of preceding and trailing whitespace. (This is public API so we need to create a new svn_cstring_split2()).</div> <div><br> </div> <div>I personally think it should be possible to specify any legal filenames in the --targets file. Files with preceding or trailing whitespace is possible under Linux. It seems NTFS also allow this but the Windows shell trim both ends, cmd.exe allow creating files with preceding whitespace. I haven't checked macOS.</div> <div><br> </div> <div>I'm less inclined to restore the original behaviour (ie option 3) since it files with preceding whitespace would still not be possible to specify in the --targets file AND we would create regressions for people who expect trailing whitespace to be trimmed.</div> <div><br> </div> <div>Thus I'm leaning towards 2, even though it may create a regression for any user of Subversion 1.1 or later. Let's call it +0.8.</div> </div> </div> </div> </div> </div> </div> </div> </div> </blockquote> <br> Before you decide to "let's do it because it's possible", think about what such file names are good for. Other than confusing people, that is, or even creating some malicious repository.<br> <br> Consider the consequences:<br> <ul> <li>Users won't see the trailing spaces in any file manager or shell.</li> <li>You can't manipulate such files at all, using the Windows command line.</li> <li>Such a change would not be backward compatible.<br> </li> </ul> <br> While you can do pretty much anything in Unix shells, the question is -- should you? I think this "use case" falls in the same category as committing files with <font face="monospace">\</font> or <font face="monospace">^H</font> (that's backspace) in their names.<br> <br> Here's an example:<br> <br> <blockquote> <pre>brane@zulu:~/src/bs$ /bin/ls -wl total 0 -rw-r--r-- 1 brane staff 0 Jun 17 15:49 foo.c -rw-r--r-- 1 brane staff 0 Jun 17 15:48 foo.c -rw-r--r-- 1 brane staff 0 Jun 17 15:49 foo.c </pre> </blockquote> <br> All three file names are unique. Which of them have trailing spaces? Hint: only one ... but hey, that's neat! We can rename all of Subversion's files and directories to look like '<font face="monospace">foo</font>' or '<font face="monospace">foo.c</font>', wouldn't that be nice?<br> <br> TL;DR: Leave well enough alone. One can already do any amount of mischief with a Unix shell, or by creating files programmatically. There's really no need to add one more avenue for confusion by "fixing" <font face="monospace">--targets</font>.<br> <br> -- Brane<br> <br> <br> <br> <br> <br> </body> </html> --------------GG0wqVioI0vb8A0dIUqcq7Rj--