[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: bring on your concerns about svn:hold, please

From: Neels J Hofmeyr <neels_at_elego.de>
Date: Tue, 23 Aug 2011 01:19:48 +0200

Thanks for talking some shop :)

I'm combining my replies to a few sub-threads in one.
At the bottom, I try to reduce the volume of this thread, so consider
skipping to just above where it says "neels: +1" near the bottom.

Mark Phippard wrote:
> When you do a merge, we could automatically remove a file from the
> ignore changelist so that it would be committed by default and not
> require the user to add a special option to their commit.

My preceding mail elaborated on this point; the local modifications may be
considered secret, so svn should not fully-automatically commit held stuff,
but should instead complain to the user.

> Right now, I think the negatives far exceed the positives.
> I think this feature could actually wind up being very
> confusing to users.

I don't see it that way at all. I don't see how negatives that don't ever
appear until you even use the feature can outweigh any positives you can get
if you choose to use the feature. I have proposed numerous solutions to all
negatives. I'm thinking: whether to use svn:hold in the repos is up to
project management. Users can still use it locally. It's always a choice.

Julian Foad wrote:
> WC-DB columns are taboo in this thread :-)
I was just saying, if people think it's too slow, there is room in that
direction. I wasn't anywhere near actually implementing that :)

I like your use case description:
  (1) An IDE configuration file (for example) is stored in the WC and it
is convenient for new checkouts to contain this file. The IDE typically
makes local changes to this file which would not be appropriate for
other users to see, such as a password or the directory to use for
temporary files. We don't want to commit these changes in a normal
commit. Occasionally, one of the developers wants to commit an
intentional modification to the config file. To do so, he/she will
revert the changes in that file that are specific to his/her username or
workstation or whatever, and carefully make and commit the desired
change. Other users expect to get any such changes when they do a
regular Subversion update. The file is assumed to be mergeable (text).

I would add that it can be useful on binary files, too.

And I need to add that the intentionally made commits on that file must
still be merged normally, and thus mergers have to take care if such changes
are part of a merge. During a merge, a user would have to discard any
non-public entries from the held-back file, like described in your use case.
(we can plaster users with warnings if they hit this case.)

> And what's this about local-diff? You intend to ignore the whole
> file that is 'held'? Or do you intend to print diffs of the parts
> that were merged [...]

I use 'svn diff' to check the local modifications that are going to be
committed. Right?? How is it obscure to omit local modifications on a
held-back file? (All mods that are already committed *are* shown.)
I would find it confusing to see changes in the diff that won't be
committed. If *all* changes are desired, as is the case during any merge
situation, again, just use --do-not-hold.

But fine, we can not affect local diff, if it is generally preferred.

> Neels wrote:
> > (With wc-copies, we should probably not copy local mods though, and
> > a diff of local mods should omit held-back stuff. But that's *all*.)
> Why that? That's a significant extra semantic change.

The only reason why I'm proposing this is to provide one more hurdle for
local secrets to leak into the repos. I am particularly arguing about a
wc-to-url copy, which would put the mods straight into a revision.

I also discussed that in notes/hold.

But, yes, we can leave that copy stuff for later, no worries at all.

> the word 'template'
This thread, linked from issue #2858, mentions an 'svn:template' prop
(roughly similar to my 'svn:hold' proposal)

Greg Stein wrote:
> ... but svn:hold? That alters the very operation of a
> version-controlled file. Now, all of a sudden... it does not
> participate in a commit. If somebody set the value *locally*, then I
> say "fine. they did it. they should know". But this thing can go into
> the repository, and then just appear in my working copy. And out of
> nowhere, my files don't commit like they should.

I think you're exaggerating. I agree with danielsh that there is no
essential difference to svn:ignore.

Each site can decide: do we allow 'svn:hold' props to be committed? If not,
pre-commit hook, done. Where is there a *real* problem here?

Committing a replace of a file can have much worse implications that are
much harder to spot ('evil twins'), yet you don't propose to purge replace
functionality from Subversion. You (probably) say: pre-commit hook, done.

> And now you're saying that it affects more than commit. Diffs? Copies?

Not necessarily. I just see that it makes a lot of sense to affect *local
changes* on held-back files during those two commands, considering wc-to-url

> If you limit it to *just* commits, and if the generated log message
> would say something like, "The following files are being held from
> commit:\nH some/path/foo \nH other/path/bar\n", then I'd be more
> supportive. When somebody goes to commit, then it is very clear these
> files are not participating. (and yes, showing 'H' in status is a good
> thing, with the understanding it means "modified, but being held"; the
> file should not show at all, if unmodified)

I agree with all of that! We *can* limit it to *just* commits. I *want* it
to print exactly that info, as you just said it. Adding hints to the
generated log message is a very good idea.

> I think 'svn diff' should continue to show the local modifications.
> And it should be copied just like any other file. And that may mean
> copying with the local mods. When committing that copy, a copy will be
> performed on the server, but the local mods will not reach the server
> due to the svn:hold.

My guess is that after a while, people will knock on the door asking why
they saw local mods in the diff that didn't get committed. I already coded
the part that omits *only* local mods from diffs on held-back files, on the
'hold' branch, but I can easily revert that again. No worries, *if* you want
that. Same goes with a wc-to-url copy: people will say "but those local mods
were *on hold*! Now my SQL password is in the history again and we have to
touch all the servers with a new password, right now, *again*! Admins will
be home late tonight, and tomorrow they *will* kill me!!"
The copy part isn't even implemented yet, so I can easily refrain from that,
leaving the kill to admins around the world ;)

Daniel Shahaf wrote:
> Random thought: We could have 'svn update' output warn when such special
> properties (svn:ignore, svn:add, svn:server-dictated-hook, svn:repository-
> dictated-config) are added by an update.

I agree with showing a warning during update:
- for adding 'svn:hold' to a locally modded file (maybe even *any* file)
- removing 'svn:hold' from local mods (maybe even unmodified files too)
- for any svn:ignore change, too, nice idea, but is a bit OT here.

But, Daniel, what does 'svn:add' do? ;) nm...

Stefan Küng wrote:
> That feature is only to prevent commits. Users use this to keep
> their local modifications from getting into the repository.
> It's basically used only for config files they need to modify
> to suit their local paths or setups.
> So no merging.

What does that mean, no merging. If someone changes the config template on
purpose, and that change is committed and is later part of a merge, that
merge should happen and be committed, too.

But I think this will probably not happen a lot. If people decided a given
file is a template, it will hardly ever see any changes committed to it.

> And they still get updates if the file gets modified in the repository -
> that's what they still want!

Thanks for that statement :)
I think we can all agree now that 'svn:hold' should not need to restrict
'update' in any way.
(I just want it to show a warning when an 'svn:hold' was deleted away from
local mods, s.a.)

So! If they want it, and if it's easy to implement, and if performance
impact is low/unnoticeable/workaroundable, and if it's *optional* (usable
local-only; and even cheaply, *optionally*, globally usable, for free, iff
project management and pre-commit permit; we could even ship with svn:hold
prohibiting code in the pre-commit template), and if Tortoise already does
something almost similar...
Then why the heck not?
What's this growing discussion actually *about*?
Auras and bikesheds?
I've really put a lot of thought in it, and there aren't any prohibitive
problems. Merge is a nut to crack but totally crackable by very simple means
(warnings for starters, and, if deemed necessary by us/you, some flag to
affect the commit after a merge touched a held-back file).
I wish more users read this list.

Sorry, had to blow off some steam ;)

Whoa, let's try to get this thread to shrink back a little.
How about just discussing "svn:hold affects *only* commit" for a start.

So if 'svn:hold' *only* affects commit; Would I eventually be allowed to
code this into trunk?

neels: +1

Such a patch is surprisingly simple, see r1159240 (for the basic approach;
naming was tweaked later).

Once that's there, an appropriate status should be printed and appropriate
warnings produced where it makes sense, as discussed above. That can follow
in the course up to 1.8. I can do these, if no-one else wants to help.

And I can stay on the branch for a while to come, too. Would just be more
convenient on trunk, cuts out all that merging.

I just need a basic go-ahead that the users' call for such a feature
should/is allowed to be answered. How minimal it will be is totally up for
discussion. I can offer more patches, and you can decline them.

At this point, I want hard fact & proof if you're going to say "no".


Received on 2011-08-23 01:20:22 CEST

This is an archived mail posted to the Subversion Dev mailing list.