You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2016/02/01 10:51:39 UTC
Re: [RFE] Make 'svn patch' read from STDIN
On Sun, Jan 31, 2016 at 11:48:26AM +0100, Andreas Scherer wrote:
> I suggest to extend 'svn patch' so that it supports usage in a pipe like
>
> gzip -dc patch-0042.gz | svn patch -P patch-0042 -
>
> This would permit using 'svn patch' directly as a drop-in replacement for
>
> gzip -dc patch-0042.gz | patch
>
> An example use case is the '%autosetup -S [SCM]' feature of RPM
> (http://www.rpm.org/wiki/PackagerDocs/Autosetup,introducedinversion4.11;
> currently supported SCM are patch, git, hg, bzr, quilt). The pipe in the
> '%autopatch' macro uses various decompressors and transfers the text output to
> the 'SCM' command. It would be nice to permit '-S svn' here as well.
Let me explain why 'svn patch' doesn't have this functionality already:
When 'svn patch' was written, for Subversion 1.7, we didn't have a
portable way to tell whether stdin is a tty. The question was how to do
this on Windows and at the time nobody involved knew how to handle that.
This has since changed, we now have the function
svn_cmdline__stdin_is_a_terminal(), as of Subversion 1.8. So we could
consider extending svn patch to run without a path argument and read the
patch from stdin.
As a first step, we'll need to discuss details of this feature's design.
To get that discussion rolling, off the top of my head I'd place the
following constraints on this feature:
The command line client would have to slurp stdin into a temporary file
before calling into the client API. The client API expects a path to the
patch file and I don't think we should change that. GUI clients have no
use case for reading patches from stdin, so the command line client would
be the only possible consumer of such an API feature. Thus, this feature
falls squarly into the domain of the command line client and not the API.
Also, I think we should preserve the following behaviour if stdin is
indeed a terminal:
$ svn patch
svn: E205001: Try 'svn help patch' for more information
svn: E205001: Not enough arguments provided
This is consistent with other svn subcommands, and avoids giving the
impression that 'svn patch' "hangs" when the user doesn't pass a patch
on stdin. This avoids confusing casual command line users line who might
not know about file redirection and pipes (I know we have such users,
based on my personal interactions with many of them).
Invoking 'svn patch' with a path to a patch file must remain a valid
use case. We cannot break backwards compatibilty here.
The help text should make clear that the 2 ways of passing a patch
are mutually exclusive. If both a path and stdin are passed, the
command should error out and show the help text.
If you like, please file an issue for this feature request at
https://issues.apache.org/jira/issues/?jql=project%20%3D%20SVN
and link to this mailing list thread in the archives from that issue.
Would you (or somebody else) be willing to work on this feature by
making the necessary changes to Subversion's code?
See http://subversion.apache.org/contributing.html#code for details.
The command line client sources are in subversion/svn/ and parts of
subversion/libsvn_subr/ (relative to a checkout of Subversion's trunk).
Re: [RFE] Make 'svn patch' read from STDIN
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Bert Huijben wrote on Sun, Feb 07, 2016 at 13:56:12 +0100:
> Well.. Perhaps ^D works on your platform, but you will need other keys
> on other platforms.
If different platforms use different keys then the message can use
#ifdef to print the right advice. I expected readers to assume that,
at the level of formality I was writing at.
>
> And ^D doesn’t abort, but signals successful EOF, so you need
> a different description else.
So we can say "exit" or "cancel" instead. (Let's figure this out when
this becomes a patch.)
Daniel
> >
> > Let's not throw the baby out with the bathwater. Simply printing an
> > informative message "svn: Reading a patch from standard input; press ^D
> > to abort" should address the confusion without breaking the use-case of
> > people who run 'patch' without arguments and then paste a patch from the
> > clipboard.
> >
> > Cheers,
> >
> > Daniel
> >
RE: [RFE] Make 'svn patch' read from STDIN
Posted by Bert Huijben <be...@qqmail.nl>.
Well.. Perhaps ^D works on your platform, but you will need other keys on other platforms. And ^D doesn’t abort, but signals successful EOF, so you need a different description else.
Creating a temporary file from the calling process/shell is most likely not much harder than in Subversion itself. And given that the diff parse code has to read the file multiple times we will need a tempfile anyway.
Bert
Sent from Outlook Mail for Windows 10 phone
From: Daniel Shahaf
Sent: zondag 7 februari 2016 01:22
To: Andreas Scherer; dev@subversion.apache.org
Subject: Re: [RFE] Make 'svn patch' read from STDIN
Stefan Sperling wrote on Mon, Feb 01, 2016 at 10:51:39 +0100:
> On Sun, Jan 31, 2016 at 11:48:26AM +0100, Andreas Scherer wrote:
> > I suggest to extend 'svn patch' so that it supports usage in a pipe like
> >
> > gzip -dc patch-0042.gz | svn patch -P patch-0042 -
> >
> > This would permit using 'svn patch' directly as a drop-in replacement for
> >
> > gzip -dc patch-0042.gz | patch
> >
> > An example use case is the '%autosetup -S [SCM]' feature of RPM
> > (http://www.rpm.org/wiki/PackagerDocs/Autosetup,introducedinversion4.11;
That link 404s; I think you meant http://www.rpm.org/wiki/PackagerDocs/Autosetup
> Also, I think we should preserve the following behaviour if stdin is
> indeed a terminal:
>
> $ svn patch
> svn: E205001: Try 'svn help patch' for more information
> svn: E205001: Not enough arguments provided
>
> This is consistent with other svn subcommands, and avoids giving the
> impression that 'svn patch' "hangs" when the user doesn't pass a patch
> on stdin. This avoids confusing casual command line users line who might
> not know about file redirection and pipes (I know we have such users,
> based on my personal interactions with many of them).
Let's not throw the baby out with the bathwater. Simply printing an
informative message "svn: Reading a patch from standard input; press ^D
to abort" should address the confusion without breaking the use-case of
people who run 'patch' without arguments and then paste a patch from the
clipboard.
Cheers,
Daniel
Re: [RFE] Make 'svn patch' read from STDIN
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Sperling wrote on Mon, Feb 01, 2016 at 10:51:39 +0100:
> On Sun, Jan 31, 2016 at 11:48:26AM +0100, Andreas Scherer wrote:
> > I suggest to extend 'svn patch' so that it supports usage in a pipe like
> >
> > gzip -dc patch-0042.gz | svn patch -P patch-0042 -
> >
> > This would permit using 'svn patch' directly as a drop-in replacement for
> >
> > gzip -dc patch-0042.gz | patch
> >
> > An example use case is the '%autosetup -S [SCM]' feature of RPM
> > (http://www.rpm.org/wiki/PackagerDocs/Autosetup,introducedinversion4.11;
That link 404s; I think you meant http://www.rpm.org/wiki/PackagerDocs/Autosetup
> Also, I think we should preserve the following behaviour if stdin is
> indeed a terminal:
>
> $ svn patch
> svn: E205001: Try 'svn help patch' for more information
> svn: E205001: Not enough arguments provided
>
> This is consistent with other svn subcommands, and avoids giving the
> impression that 'svn patch' "hangs" when the user doesn't pass a patch
> on stdin. This avoids confusing casual command line users line who might
> not know about file redirection and pipes (I know we have such users,
> based on my personal interactions with many of them).
Let's not throw the baby out with the bathwater. Simply printing an
informative message "svn: Reading a patch from standard input; press ^D
to abort" should address the confusion without breaking the use-case of
people who run 'patch' without arguments and then paste a patch from the
clipboard.
Cheers,
Daniel
Re: [RFE] Make 'svn patch' read from STDIN
Posted by Andreas Scherer <an...@freenet.de>.
Am Montag, 1. Februar 2016, 13:01:58 schrieb Stefan Sperling:
> What does -P do? I cannot seem to find it in documentation for BSD patch,
> GNU patch, git apply, and svn patch.
Mwa culpa: this option belongs to 'quilt' and is not related to 'svn patch'.
Re: [RFE] Make 'svn patch' read from STDIN
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Feb 01, 2016 at 12:44:38PM +0100, Andreas Scherer wrote:
> Am Montag, 1. Februar 2016, 10:51:39 schrieb Stefan Sperling:
> > So we could consider extending svn patch to run without a path argument
> > and read the patch from stdin.
> >
> > [...] this feature falls squarly into the domain of the command line client
> > and not the API.
> >
> > Also, I think we should preserve the following behaviour
> > $ svn patch
> > svn: E205001: Try 'svn help patch' for more information
> > svn: E205001: Not enough arguments provided
> > Invoking 'svn patch' with a path to a patch file must remain a valid
> > use case. We cannot break backwards compatibilty here.
>
> > > gzip -dc patch-0042.gz | svn patch -P patch-0042 -
> That's why my suggested invocation had a hyphen '-' at the end, playing the
> role of a place-holder for 'stdin'. This preserves both current situations,
> i.e., 'svn patch' being called without an argument (--> error) and with a
> filename (--> normal processing).
Sure. My idea was to auto-detect based on stdin being tty or file
instead of requiring the user to pass a hyphen to indicate that
stdin should be used. But either approach should work.
> Also, 'svn patch -' should _require_ to be
> supplied with the '-P' option and a filename for storage.
What does -P do? I cannot seem to find it in documentation for BSD patch,
GNU patch, git apply, and svn patch.
Re: [RFE] Make 'svn patch' read from STDIN
Posted by Andreas Scherer <an...@freenet.de>.
Am Montag, 1. Februar 2016, 10:51:39 schrieb Stefan Sperling:
> So we could consider extending svn patch to run without a path argument
> and read the patch from stdin.
>
> [...] this feature falls squarly into the domain of the command line client
> and not the API.
>
> Also, I think we should preserve the following behaviour
> $ svn patch
> svn: E205001: Try 'svn help patch' for more information
> svn: E205001: Not enough arguments provided
> Invoking 'svn patch' with a path to a patch file must remain a valid
> use case. We cannot break backwards compatibilty here.
> > gzip -dc patch-0042.gz | svn patch -P patch-0042 -
That's why my suggested invocation had a hyphen '-' at the end, playing the
role of a place-holder for 'stdin'. This preserves both current situations,
i.e., 'svn patch' being called without an argument (--> error) and with a
filename (--> normal processing). Also, 'svn patch -' should _require_ to be
supplied with the '-P' option and a filename for storage.
Thanks for considering this.