On Mon, Feb 08, 2010 at 04:55:24PM +0100, Ulrich Eckhardt wrote:
> On Monday 08 February 2010, Stefan Sperling wrote:
> > > 2. Target being a non-working copy.
> > > I don't remember where I wanted to use this one, I believe I wanted to
> > > apply changes I made to an SVN vendor branch to a (non-versioned) release
> > > of that vendor or to some code in a completely different VCS.
> >
> > Why not use the normal patch command then?
>
> Lazyness. Perhaps also because patch can't handle a few SVN-specific things
> like tree changes or diffs in non-text files, at least not as well as SVN
> does.
svn patch won't handle tree changes either if the target is not a working copy.
The tree operations are supported because svn supports them (and has to
be told about them as you carry them out, svn add, svn copy, svn rm,
etc.) When you move a file in the file system, without having a VCS beneath,
a copy is really just an add. A move is really just a copy + delete.
As far as non-text files go, svn diff, as well as normal diff, just
print something to the effect of "this file has changed". svn diff and
svn patch do not handle binary files.
So, again, in the non-working-copy case, svn patch has no advantage
over normal patch.
> > We're not trying to replace the normal patch command. We're creating an
> > augmented patch implementation for use with svn working copies.
> > svn patch is geared towards svn working copies. E.g. it won't know how
> > to add or delete files in a working copy of a different VCS, so it does
> > not offer any benefits over the traditional patch command for this use
> > case.
>
> Well, even if it doesn't know how to tell the VCS about the added file, it
> could at least add/delete/move the file and leave the rest to the user. I
> don't expect SVN to do all work for me, but if it can apply a patch to a
> working copy, why not to a non-working copy if the user asks for it? Yes, I
> can probably do the same by piping "svn diff" into "patch", but firstly I
> don't always have patch, secondly I can never remember its syntax and thirdly
> it is of limited use sometimes.
svn patch as it is now has more limitations than normal patch.
For example, in a working copy, you have backups for free (you can use
svn revert). Normal patch has to make backup files to avoid causing
irreversible damage. So this would be not just a trivial enhancement
to svn patch, it would really be a second mode of operation.
And what is the real advantage over not having to use the normal patch
program? From what you're saying the only real advantage would be that people
using systems where patch does not come pre-installed (like windows) need
only install one tool instead of two. That argument doesn't seem really
convincing.
Anyway, the idea itself is not entirely unreasonable, and I've talked
about it to other developers briefly. Short version is that we're not
really sure if the idea is good or not. See below.
Stefan
<stsp> the code would be full of if (working copy) do this else do that
<stsp> at the user interface, we could add a --no-working-copy flag
<dannas> It feels wrong. We set out to loose the dependence on patch(1)
and now we're talking about replacing patch(1) for general cases?
<stsp> the users asking for this are on windows
<dannas> ah, no patch(1).
<dannas> still feels wrong.
<stsp> sure it does
<stsp> question is whether we should do them a favour or not
<stsp> it's not hard to do
<julianf> In general it feels wrong to make Subversion be a substitute
for other non-VCS tools. However, the functionality of "patch" is
something that makes sense in a non-VCS context (at least until
properties come into play), and so maybe it does logically make sense to
support it.
<stsp> one problem is that when operating on a WC, we're
not destructive -- svn revert can undo things
<stsp> in a non-WC, that's not possible
<stsp> so we'd either have to tell users to live with that, or add even
more features from standard patch such as backing up files
<julianf> (And while we're here, let's do "svn diff" also. I have in the
past thought that "svn diff" should be general enough to diff two
unversioned files/dirs. It's useful to be sure I'm applying the same
diff algorithm to some as-yet-unversioned files that I'm using on my
versioned files.)
<stsp> if we're doing this for both diff and patch, i'd say postpone to 1.8
<julianf> :-)
<julianf> I'm just throwing the idea into the mix.
<stsp> i like it, also for symmetry
<stsp> i'd say if we do one, we do the other, too
<julianf> Yes, good policy.
<julianf> stsp, dannas: Patch-to-non-WC probably doesn't need an
explicit switch. If the destination is a WC, fine, apply it. If it is
not a WC, fine, apply it, but warn if the patch includes any
prop-changes because those are being ignored.
<stsp> julianf, I don't want to destroy data
<stsp> it should error out by default, and ask for a --no-working-copy
switch before operating on a non-WC tree
<stsp> otherwise I won't any longer be able to sleep at night :)
<julianf> But, all that said, it might be a very good idea to NOT go
implementing a whole extra chunk of functionality, however little code
it might require, because it will bring its own set of bugs and
undesired edge cases.
<julianf> (I think it's much too early to say whether, if the non-WC
feature were to become reality, it should require a --no-working-copy
switch.)
<julianf> stsp: I know, you could implement it internally, but not
expose it yet, especially if starting to support non-WC application
doesn't mess up (and even helps clean up) the internal design, which it
might.
<stsp> it will be messier internally for sure
<stsp> more special cases, not less
<stsp> you suddenly need to ask yourself if you are in a working copy or
not before doing certain things
<julianf> Yeah, I guess...
<julianf> stsp: Another reason is (like I said for "svn diff" above)
being sure you can get the same outputs from the same inputs, on your
non-WC versus your WC. It's icky if one tool only supports one case,
another tool only supports the other, and they behave slightly
differently.
<stsp> julianf, good point
<julianf> However, it's also icky to re-invent (sorry, re-implement) an
external tool in the first place. I feel queasy about "diff" and "patch"
being implemented in svn - we'll always be playing feature catch-up with
them, diverting from our main (version control) tasks. But I acknowledge
it's useful to have them built in.
Received on 2010-02-08 19:46:59 CET