On 08/22/2011 04:45 PM, Mark Phippard wrote:
> On Mon, Aug 22, 2011 at 10:32 AM, Neels J Hofmeyr <neels_at_elego.de
> <mailto:neels_at_elego.de>> wrote:
>
> Greg/anyone, do you have any more arguments against svn:hold except "people
> will trip over a prop they read the documentation of and set up themselves
> before it had any effect" -- because I don't think that's a real problem.
> They would have tripped over 'svn:ignore', 'svn:externals' and
> 'svn:keywords' long before tripping over 'svn:hold'. BTW, my intention is to
> clearly mark held-back local mods in 'svn status' (e.g. 'H'); particularly
> important for merge situations.
>
> In my vision, 'svn:hold' would be the exception, in the scale of one
> build.conf type of file per huge project tree. But that's up to the users.
> Which is exactly the point!
>
> (See notes/hold for more points.)
>
>
>
> I really do not know where I stand on this. If you had asked me before you
> started if it would be good to have a feature where certain local files
> could be ignored by commit I would have said Yes. Now that you have started
> working on it, and some more thought has gone into it I am less sure. I do
> worry about the implications of a feature like this in terms of what the
> behavior should be on other commands.
>
> Maybe a different implementation could make a difference? Just thinking out
> loud ....
>
> What if SVN had a feature like TortoiseSVN where any file in a specifically
> named changelist was automatically ignored on commit unless that specific
> changelist is named ($ svn ci -cl ignored-files ) We could probably make
> commands like status and diff automatically ignore this changelist too.
Hi Mark,
you first express concerns about implications on other commands, and then
you propose the exact same feature and implications using changelists? Come
on :)
>
> Perhaps we could then go a step farther with this feature by automatically
> adding files with a special property like svn:hold to this changelist when
> they are checked out?
I thought about this too, actually, and came to this conclusion: it just
inflates the whole implementation. Nothing more. The exact same thing can be
achieved with *just* a property, without having to "rape" changelists: the
changes to changelists would be: introduce special treatment of changelists
that have a specific name (-1). Introduce reversable changelists (+1) but
only do it implicitly, if it has one certain name (-1). *After* you did
this, you put into life a property (where we already have a reserved
namespace and long history for storing svn-internals), and instead of
querying the prop directly, you have changelists inserted in the chain. I'm
-1 to that.
Also realize: having an implicit negative changelist is basically the same
as having the svn:hold property set locally. It doesn't get committed
(unless you force it) so it's like a WC-specific config, like a negative
changelist. But you get the option to easily commit and send it to other
users almost for free, just add the --do-not-hold option (an option that
should anyway exist, even for a negative changelist).
If I were to decide, svn:hold would act by default as soon as it is present,
but I'd also be fine to force users to pass a --do-hold option or having to
set a ~/.subversion/conf item before their clients heed svn:hold props.
*Then* it would be *exactly* what you just proposed, just a lot cleaner to
implement (not introducing changelists exceptions), with the option of
allowing that prop to go through to other WCs directly.
>
> Maybe if svn merge updates a file with this property it removes it from the
> changelist so that it will be in the list of files to commit? svn commit
> could add the file back to the changelist, just as a commit on a locked file
> puts the read-only attribute back on the file?
That sounds overly complicated to me.
How do you remember which of the files 'svn commit' should add back to the
changelist? I'd rather have an explicit NODES-table column for that,
changelists or no. FWIW, 'merge' could actually add a temporary 'svn:merged'
prop next to the 'svn:hold' prop to warn 'svn commit'.
And for the use case where people want to prevent secrets from being
committed, this could be very dangerous. Switching off blocking behaviour
should thus have to be done explicitly by the user, to acknowledge that no
secrets exist in the current local modifications. That's why I'd only go as
far as commit-after-a-merge refusing to go ahead until
--do-not-hold/--do-hold (or similar) is passed explicitly.
>
> An approach like this might be more complicated to implement but it avoids
> adding new options,
I disagree with not needing new options, and I don't think that new options
are a bad thing, either ;)
And I agree that it's more complicated :)
> and I *think* it makes the behavior on other commands
> clearer.
I don't really see that. No matter how this holding back thing were
implemented, it would need to have the behavior I have outlined, and it
would affect:
commit,
local-diff,
wc-to-* copy,
merge (print warning to add --do-not-hold to the next commit
OR set flag),
update/switch (print warning if prop gets removed,
iff there are local mods aka secrets).
These requirements do not change when you do it with changelists.
> Since TortoiseSVN has had this feature for a long time, it would
> be interesting to know what kind of edge cases people have raised with the
> feature. Has anyone asked that these files not be updated or merged?
I've asked, but only on this list... good point, I should ask tortoise devs
directly. (But their changelist is called "ignore-on-commit", so it's pretty
clear what it does, I guess.)
If we had a simple --exclude <glob> option to 'commit', maybe we wouldn't be
having this discussion today. There is to-date no easy way to tell svn to
skip single files (svn has no negatively acting options for this case).
So if svn:hold is defied, or maybe on top of it, too, I'd try to implement
such an option. But I still prefer svn:hold, because it adds the ability for
the senior developers to set default behavior in the WCs of junior
developers (yes, we are writing a centralized VCS, and many users want
centralizable config), and because you don't have to perpetually remember to
pass an --exclude option for the right WCs, at times where you're in the
middle of hacking on a patch.
Wow, I didn't think I'd have to argue this much :)
I still don't see how 'svn:hold' is problematic to include in 1.8.
More cons?
~Neels
Received on 2011-08-22 18:51:55 CEST