You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Peter Samuelson <pe...@p12n.org> on 2006/03/15 00:51:35 UTC

automatic keyword un-expansion on 'svn {ps,pd,pe} svn:keywords'?

Christof Douma points out that "svn pdel svn:keywords" does not
un-expand a file's keywords in the working copy.  He thinks it should,
and I tend to agree.  Present behavior leaves a working copy diff due
to keyword expansion, even if the file is otherwise unchanged.

(He also reports a bug in 1.2.3, but I verified that his bug is not in
1.3.0 or trunk.  See http://bugs.debian.org/356900 for his detailed and
lucid report.)

Comments?  Please Cc: 356900@bugs.debian.org in replies.

Thanks,
Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: automatic keyword un-expansion on 'svn {ps,pd,pe} svn:keywords'?

Posted by Julian Foad <ju...@btopenworld.com>.
Peter Samuelson wrote:
> [Julian Foad]
> 
>>Finally, but importantly, why are we only talking about contracting 
>>keywords?
> 
> We're talking about any change to the svn:keywords property - both
> adding and removing keywords.

The original post on the Subversion dev list did not mention adding, but it's 
good that we are talking about it now.

>  For the add case, libsvn_wc already does
> the right thing and expands keywords in the WC (though at commit time,
> not at propset/propedit time).  Doing it at propset/propedit time is
> probably not worthwhile, since (at least for $Id$) the string will
> change at commit time anyway.
> 
> 
>>Surely, if we expect 'svn' to contract a keyword when we remove its
>>name from the property, we should also expect it to expand it when we
>>insert its name in the property.
> 
> Doesn't that happen today?  Or do you mean doing it immediately rather
> than deferring it to the next commit?

I meant doing it immediately, and I was asking because I thought that's what 
was being proposed for contracting a deleted keyword, but I hope (and it now 
seems) that's not the case.


As far as I can tell, we're pretty much in agreement that the current behaviour 
is right.  The fundamental point is:

   A keyword absent from 'svn:keywords' does NOT mean that Subversion
   should contract it, it means that Subversion should not touch it
   nor even require that it parses syntactically.

So we're left with the lesser question:

   * Should we provide an easy one-step facility to contract and then ignore a 
keyword?

My answer: no, I don't see any good reason for doing so.  I recommend the bug 
report be closed as "fixed", since the definite bug (in diff) noted in the 
original Debian report has been fixed and I don't think further action is required.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: automatic keyword un-expansion on 'svn {ps,pd,pe} svn:keywords'?

Posted by Peter Samuelson <pe...@p12n.org>.
[Julian Foad]
> > True.  The only frustrating thing is that you can't use 'svn
> > revert' to get rid of the local mod.
> 
> Did you mean, "you can't use 'svn revert' to contract the keyword" ?

Yeah, you can't use svn revert to get rid of the local diff because
that also gets rid of the keyword change.

It's easy enough to work around, using 'svn diff | patch -R', assuming
a sufficiently sophisticated user.  So I'm not really worried about it.


> What happens if there is an expanded keyword in the repository (which
> can easily happen if it wasn't mentioned in the 'svn:keywords'
> property at first, but is mentioned later)?  In this situation, if
> the 'svn:keywords' property was set by a hypothetical remote-propset
> command, what would then happen at the client, or what would we want
> to happen?

I would expect the server to continue to store every active $Keyword$
in the canonical collapsed form, as it does now.  But I suppose the
logic for this is entirely client-side.  A direct mode 'svn prop*'
family of commands would imply having to put such logic in the server,
which seems unfortunate.  (I mean particularly for the case of _adding_
a keyword - the server would want to collapse instances of the keyword
in the affected file.)

> Finally, but importantly, why are we only talking about contracting 
> keywords?

We're talking about any change to the svn:keywords property - both
adding and removing keywords.  For the add case, libsvn_wc already does
the right thing and expands keywords in the WC (though at commit time,
not at propset/propedit time).  Doing it at propset/propedit time is
probably not worthwhile, since (at least for $Id$) the string will
change at commit time anyway.

> Surely, if we expect 'svn' to contract a keyword when we remove its
> name from the property, we should also expect it to expand it when we
> insert its name in the property.

Doesn't that happen today?  Or do you mean doing it immediately rather
than deferring it to the next commit?

Re: automatic keyword un-expansion on 'svn {ps,pd,pe} svn:keywords'?

Posted by Julian Foad <ju...@btopenworld.com>.
Peter Samuelson wrote:
> [kfogel@collab.net]
> 
>>Hmmm.  What you say makes me think that showing a "local mod" (the
>>still-expanded keyword) after the propdel might be at least as useful
>>as automatically contracting the keyword.  And showing the local mod
>>alerts the user that there's a conscious decision to be made here.
> 
> 
> True.  The only frustrating thing is that you can't use 'svn revert' to
> get rid of the local mod.

'svn revert' returns the file to a locally unmodified state with no diff, 
because it reverts the deletion of the svn:keywords property:

$ echo '$Rev$' > kw
$ svn add kw
$ svn ps svn:keywords rev kw
$ svn ci -m '' kw
$ svn pd svn:keywords kw
$ svn diff
Index: kw
===================================================================
--- kw  (revision 1359)
+++ kw  (working copy)
@@ -1 +1 @@
-$Rev$
+$Rev: 1359 $

Property changes on: kw
___________________________________________________________________
Name: svn:keywords
    - rev

$ svn revert kw
Reverted 'kw'
$ svn diff
[no output]


Did you mean, "you can't use 'svn revert' to contract the keyword" ?  That's 
certainly true, but that seems logical to me, or at least I don't see any 
particular reason why it should do so.  If you're stopping using the keyword, 
who's to say what you want the text in your file to look like?


>  You're left with 'svn diff {f} | patch -R'
> or recopying the text-base file manually.
> 
> I tried to do a repository-side 'svn propdel' and of course that
> doesn't exist, for reasons I only remembered afterwards.  If it did
> exist, though, obviously it *would* unexpand the keywords, and thus
> would make an argument for WC-side 'svn propdel' doing the same.

I see what you're getting at, and there certainly is some sort of inconsistency 
there, but I'm not entirely convinced by your argument.  It wouldn't "unexpand" 
the keyword in the repository file, it would just result in the keyword not 
being expanded by the client, which is subtly different.

What happens if there is an expanded keyword in the repository (which can 
easily happen if it wasn't mentioned in the 'svn:keywords' property at first, 
but is mentioned later)?  In this situation, if the 'svn:keywords' property was 
set by a hypothetical remote-propset command, what would then happen at the 
client, or what would we want to happen?  I'm just asking about this opposite 
case in the hope that thinking about it will shed some light on the case that 
is the subject of this thread.

Finally, but importantly, why are we only talking about contracting keywords? 
Surely, if we expect 'svn' to contract a keyword when we remove its name from 
the property, we should also expect it to expand it when we insert its name in 
the property.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: automatic keyword un-expansion on 'svn {ps,pd,pe} svn:keywords'?

Posted by Vincent Lefevre <vi...@vinc17.org>.
On 2006-03-15 15:36:48 -0600, kfogel@collab.net wrote:
> The repository wouldn't exactly unexpand the keywords, since they're
> not stored expanded anyway.  It could just cause future updates to
> unexpand them in a working copy, which would be a case where receiving
> a prop-only modification *would* affect the text of a file.

Note that this is already the case: modifying a property *sometimes*
changes the last-modification date (there's bug 1743 about that).

-- 
Vincent Lefèvre <vi...@vinc17.org> - Web: <http://www.vinc17.org/>
100% accessible validated (X)HTML - Blog: <http://www.vinc17.org/blog/>
Work: CR INRIA - computer arithmetic / SPACES project at LORIA

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: automatic keyword un-expansion on 'svn {ps,pd,pe} svn:keywords'?

Posted by kf...@collab.net.
Peter Samuelson <pe...@p12n.org> writes:
> [kfogel@collab.net]
> > Hmmm.  What you say makes me think that showing a "local mod" (the
> > still-expanded keyword) after the propdel might be at least as useful
> > as automatically contracting the keyword.  And showing the local mod
> > alerts the user that there's a conscious decision to be made here.
> 
> True.  The only frustrating thing is that you can't use 'svn revert' to
> get rid of the local mod.  You're left with 'svn diff {f} | patch -R'
> or recopying the text-base file manually.
> 
> I tried to do a repository-side 'svn propdel' and of course that
> doesn't exist, for reasons I only remembered afterwards.  If it did
> exist, though, obviously it *would* unexpand the keywords, and thus
> would make an argument for WC-side 'svn propdel' doing the same.

Wow.

I truly do not know what the right answer here is, if there is one,
which there clearly isn't.

The repository wouldn't exactly unexpand the keywords, since they're
not stored expanded anyway.  It could just cause future updates to
unexpand them in a working copy, which would be a case where receiving
a prop-only modification *would* affect the text of a file.  Let's
see, according to the script below run with head-of-trunk SVN... Nope!
Updating to receive the removal of the property doesn't affect the
text of the working file either:

--------------------8-<-------cut-here---------8-<-----------------------
#!/bin/sh

SVN=${HOME}/src/subversion/subversion/svn/svn
SVNADMIN=${HOME}/src/subversion/subversion/svnadmin/svnadmin

URL=file:///`pwd`/repos

rm -rf repos wc wc-other

${SVNADMIN} create repos
${SVN} co -q ${URL}/ wc

cd wc
echo '$Revision$' >> newfile
${SVN} add -q newfile 
${SVN} propset -q svn:keywords "rev" newfile
${SVN} ci -q -m "Add newfile, with keywords property."
echo ""
echo "The property has been set.  The keyword should be expanded:"
echo ""
cat newfile
cd ..

# Unset the prop from another working copy.
${SVN} co -q ${URL} wc-other
cd wc-other
${SVN} propdel -q svn:keywords newfile
${SVN} ci -q -m "Remove keywords property from newfile."
cd ..

# Update the original working copy.
cd wc
${SVN} update -q
echo ""
echo "The property has been updated away.  What happened to the keyword?:"
echo ""
cat newfile


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: automatic keyword un-expansion on 'svn {ps,pd,pe} svn:keywords'?

Posted by Peter Samuelson <pe...@p12n.org>.
[kfogel@collab.net]
> Hmmm.  What you say makes me think that showing a "local mod" (the
> still-expanded keyword) after the propdel might be at least as useful
> as automatically contracting the keyword.  And showing the local mod
> alerts the user that there's a conscious decision to be made here.

True.  The only frustrating thing is that you can't use 'svn revert' to
get rid of the local mod.  You're left with 'svn diff {f} | patch -R'
or recopying the text-base file manually.

I tried to do a repository-side 'svn propdel' and of course that
doesn't exist, for reasons I only remembered afterwards.  If it did
exist, though, obviously it *would* unexpand the keywords, and thus
would make an argument for WC-side 'svn propdel' doing the same.

Peter

Re: automatic keyword un-expansion on 'svn {ps,pd,pe} svn:keywords'?

Posted by kf...@collab.net.
Julian Foad <ju...@btopenworld.com> writes:
> I'll just mention a few points.
> 
> When a user asks Subversion to stop handling a keyword,
> 
>   * some will want to remove the entire keyword from the text;
> 
>   * some will want to keep the last expanded value as ordinary versioned text;
> 
>   * some will want to keep an empty, contracted keyword placeholder in the text.
> 
> The first requires manual editing, and that's fine.  Of the second and
> third, do we have a hope of guessing which is more common?
> 
> Is it "svn diff" that we should be fixing, rather than "propdel"?
> 
> Consider the dangers of modifying a file while it might be open in a
> user's editor.  The same dangers apply to "update", "merge", etc., so
> it's "just" a matter of user education, but text changes caused by a
> property command is a new phenomenon.
> 
> If we are going to contract the keyword, consider doing it at commit
> time rather than at prop-change time, as that would avoid the
> text-change problem. (Do what, exactly?  Contract the set of keywords
> that is the union of base and working prop values?)
> 
> More thought required.

Hmmm.  What you say makes me think that showing a "local mod" (the
still-expanded keyword) after the propdel might be at least as useful
as automatically contracting the keyword.  And showing the local mod
alerts the user that there's a conscious decision to be made here.

My (mild) inclination is to stay with status quo now.  The diff may be
annoying, but at least it's not a silent change.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: automatic keyword un-expansion on 'svn {ps,pd,pe} svn:keywords'?

Posted by Julian Foad <ju...@btopenworld.com>.
Peter Samuelson wrote:
> Christof Douma points out that "svn pdel svn:keywords" does not
> un-expand a file's keywords in the working copy.  He thinks it should,
> and I tend to agree.  Present behavior leaves a working copy diff due
> to keyword expansion, even if the file is otherwise unchanged.

I'll just mention a few points.

When a user asks Subversion to stop handling a keyword,

  * some will want to remove the entire keyword from the text;

  * some will want to keep the last expanded value as ordinary versioned text;

  * some will want to keep an empty, contracted keyword placeholder in the text.

The first requires manual editing, and that's fine.  Of the second and third, 
do we have a hope of guessing which is more common?

Is it "svn diff" that we should be fixing, rather than "propdel"?

Consider the dangers of modifying a file while it might be open in a user's 
editor.  The same dangers apply to "update", "merge", etc., so it's "just" a 
matter of user education, but text changes caused by a property command is a 
new phenomenon.

If we are going to contract the keyword, consider doing it at commit time 
rather than at prop-change time, as that would avoid the text-change problem. 
(Do what, exactly?  Contract the set of keywords that is the union of base and 
working prop values?)

More thought required.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: automatic keyword un-expansion on 'svn {ps,pd,pe} svn:keywords'?

Posted by John Peacock <jp...@rowman.com>.
kfogel@collab.net wrote:
> I think that's reasonable.  Having a propdel result in a textual diff
> is rather unexpected, and in some sense the in-file expanded portion
> of the keyword is "part of" the property.  We'll have to do it
> carefully, of course, because the working file might have local mods.

CAVEAT: Not a developer, but someone interested in keywords in general.

Personally I think it would be _much_ more surprising to automatically 
unexpand the keyword when the matching property is deleted.  If someone 
sets one of the expanding properties and the unsets it, either they made 
a mistake in the first place (and should revert that expanded text on 
their own) or they meant to do that to freeze the expanded text (and 
hence the diff is completely legit).

For example, a site policy is that all new files are to be created with 
svn:keywords "Author" set and then committed.  Then, the svn:keywords 
"Author" is to be removed and the file committed again, thus orphaning 
the original Author information in the file for all time.  Sure this is 
stupid, but it does correspond to some behavior that has been used in 
CVS in the past (although there I believe it was freezing $Id$ on tags).

My 2 cents

John

-- 
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4501 Forbes Boulevard
Suite H
Lanham, MD  20706
301-459-3366 x.5010
fax 301-429-5748

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Bug#356900: automatic keyword un-expansion on 'svn {ps,pd,pe} svn:keywords'?

Posted by kf...@collab.net.
Peter Samuelson <pe...@p12n.org> writes:
> Christof Douma points out that "svn pdel svn:keywords" does not
> un-expand a file's keywords in the working copy.  He thinks it should,
> and I tend to agree.  Present behavior leaves a working copy diff due
> to keyword expansion, even if the file is otherwise unchanged.
> 
> [...]
> 
> Comments?  Please Cc: 356900@bugs.debian.org in replies.

I think that's reasonable.  Having a propdel result in a textual diff
is rather unexpected, and in some sense the in-file expanded portion
of the keyword is "part of" the property.  We'll have to do it
carefully, of course, because the working file might have local mods.

+1 from me, but I'd like to hear at least one other developer come to
the same conclusion before we implement it, as this is a somewhat
noticeable behavior change.

-Karl

-- 
www.collab.net  <>  CollabNet  |  Distributed Development On Demand


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: automatic keyword un-expansion on 'svn {ps,pd,pe} svn:keywords'?

Posted by kf...@collab.net.
Peter Samuelson <pe...@p12n.org> writes:
> Christof Douma points out that "svn pdel svn:keywords" does not
> un-expand a file's keywords in the working copy.  He thinks it should,
> and I tend to agree.  Present behavior leaves a working copy diff due
> to keyword expansion, even if the file is otherwise unchanged.
> 
> [...]
> 
> Comments?  Please Cc: 356900@bugs.debian.org in replies.

I think that's reasonable.  Having a propdel result in a textual diff
is rather unexpected, and in some sense the in-file expanded portion
of the keyword is "part of" the property.  We'll have to do it
carefully, of course, because the working file might have local mods.

+1 from me, but I'd like to hear at least one other developer come to
the same conclusion before we implement it, as this is a somewhat
noticeable behavior change.

-Karl

-- 
www.collab.net  <>  CollabNet  |  Distributed Development On Demand

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org