You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2007/08/18 02:31:33 UTC

If no such changelist, show we error?

The following IRC conversation says it all:

   <kfogel> Is it a deliberate decision that 'svn commit -m logmsg
            --changelist NON_EXISTENT_CHANGELIST_NAME' does nothing?
            (It doesn't show any error, and it doesn't commit anything,
            even if there are local mods, though of course those local
            mods are not in the named changelist, since it doesn't exist.)
   
   <kfogel> This behavior is true even if you pass --keep-changelist
   
   <epg>    i vote bug; it should error with "no such changelist"
   
   <kfogel> epg: I agree.
   
   <kfogel> Hmm, I'll ask dev@, just in case there was some esoteric
            reason why this behavior was deliberate.

-Karl

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

Re: If no such changelist, show we error?

Posted by Talden <ta...@gmail.com>.
> > If changelists are intended to hold paths rather than scheduled
> > changes then I'd have expected items to stay on changelists and for
> > changelists to be a feature of the working copy as a whole - allowing
> > a path to be listed on multiple changelists
>
> Er, um... you really don't want a path to be part of 2 changelists at
> once.  That would imply two overlapping patches, and 'svn ci
> ---changelist cl1' wouldn't know which 'hunks' of patches to commit.
> This was a scenario we deliberately decided to avoid when we designed
> this fetaure.

Treating them 'as a patch' means that a changelist really _is_ 'a set
of scheduled changes' rather than a 'list of paths'.  In that sense, a
commit of a path should _always_ clear the path from any changelist as
that change has been committed and cannot still be scheduled - it no
longer forms part of that patch.

Another alternative is to abort any commit that does not name a
changelist but contains 1 or more modified paths on changelists.  This
way the user cannot accidentally break up a patch by failing to
specify the changelist and thus breaking up the patch they've already
defined. If the user doesn't use changelists this doesn't change
behaviour from pre changelist versions - but if they do it gives them
better protection from user error.

--
Talden

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

Re: If no such changelist, show we error?

Posted by Ben Collins-Sussman <su...@red-bean.com>.
On 8/19/07, Talden <ta...@gmail.com> wrote:
> That is, "svn diff --changelist
> > foo" may not show all of the changes conceptually on changelist "foo";
> > instead, it shows those changes that are at or below the current
> > directory.
>
> Equally unintuitive but understandable given the WC model.
>

Luckily, we designed the API for changelists in such a way such that
they *could* live in a centralized database.  After some list
discussion, we decided to do this so that a better libsvn_wc 2.0
really could return all paths in a CL, not just the ones below the
current working directory.

> > On the other hand, I think it's rather unexpected
> > that "svn ci foo.c" removes foo.c from any changelist it's on even
> > though changelists aren't being mentioned at all.

Yeah, that does seem weird to me.  I think we should fix that, or file
it as a bug.


> If changelists are intended to hold paths rather than scheduled
> changes then I'd have expected items to stay on changelists and for
> changelists to be a feature of the working copy as a whole - allowing
> a path to be listed on multiple changelists

Er, um... you really don't want a path to be part of 2 changelists at
once.  That would imply two overlapping patches, and 'svn ci
---changelist cl1' wouldn't know which 'hunks' of patches to commit.
This was a scenario we deliberately decided to avoid when we designed
this fetaure.


> and for changelists to be
> persistent after commits (unless a user specified option at
> commit-time cleared the paths from the specified list) would allow for
> a more natural usage if changelists really are lists of paths rather
> than scheduled changes.

We're deliberately modeling perforce here.  That's why the default
behavior is for 'svn commit' to destroy changelists, rather than keep
them around.  It's also consistent with the way our locking feature
works.  ('svn commit' uses a lock to do the commit, then automatically
releases the lock by default.)

>
> I think a major review of many of these decisions (and unfortunately a
> shift in the meaning of some concepts for the user) will be necessary
> when/if the fundamental centralisation of working copy book-keeping is
> ever done.

Agreed.

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

Re: If no such changelist, show we error?

Posted by Talden <ta...@gmail.com>.
> > > > cl has committables:                commit them
> > > > cl has files but no committables:   silently exit 0
> > > > cl has no files:                    silently exit 0
> > > > cl does not exist:                  error out
> > >
> > > In the current implementation, this distinction does not exist.  There
> > > is no mapping from changelists to entries; rather, each wc entry may
> > > have a changelist name.
> >
> > Ah, I guess that's why it behaves this way now.  It's not really
> > a list, but a label or tag that files may nor may not have.  Hm.
>
> Indeed.  In fact, the nature of the changelist leads to a couple of
> other behaviors which I find unintuitive as a user (though sensible
> once you understand the current implementation):

Very unintuitive but also completely understandable with a brief explanation.

> As we discussed on IRC, svn commands never "recurse up" from the
> highest ancestor of the files listed on the command line (or the
> working directory if no files are listed).  This means that the files
> referred to by any --changelist command (other than commit, which is
> special) depends on the current wc.  That is, "svn diff --changelist
> foo" may not show all of the changes conceptually on changelist "foo";
> instead, it shows those changes that are at or below the current
> directory.

Equally unintuitive but understandable given the WC model.

> Another violation of a user's expectation that a changelist is in some
> way atomic is the changelist-unsetting behavior of commit.  I like the
> behavior that "svn ci --changelist bla" removes the items from the
> "bla" changelist afterwards, and that you can use --keep-changelist to
> keep it around.  On the other hand, I think it's rather unexpected
> that "svn ci foo.c" removes foo.c from any changelist it's on even
> though changelists aren't being mentioned at all.
>
> Does anyone else find this properties of the current changelist
> design/implementation surprising?

If changelists are intended to hold paths rather than scheduled
changes then I'd have expected items to stay on changelists and for
changelists to be a feature of the working copy as a whole - allowing
a path to be listed on multiple changelists and for changelists to be
persistent after commits (unless a user specified option at
commit-time cleared the paths from the specified list) would allow for
a more natural usage if changelists really are lists of paths rather
than scheduled changes.

I think a major review of many of these decisions (and unfortunately a
shift in the meaning of some concepts for the user) will be necessary
when/if the fundamental centralisation of working copy book-keeping is
ever done.

--
Talden

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

Re: If no such changelist, show we error?

Posted by David Glasser <gl...@davidglasser.net>.
On 8/18/07, Eric Gillespie <ep...@pretzelnet.org> wrote:
> "David Glasser" <gl...@davidglasser.net> writes:
>
> > On 8/18/07, Eric Gillespie <ep...@pretzelnet.org> wrote:
> > > No, if the changelist doesn't exist, I agree we should error.  I
> > > was disagreeing that we should error if the cl does exist, but is
> > > just empty.
> > >
> > > cl has committables:                commit them
> > > cl has files but no committables:   silently exit 0
> > > cl has no files:                    silently exit 0
> > > cl does not exist:                  error out
> >
> > In the current implementation, this distinction does not exist.  There
> > is no mapping from changelists to entries; rather, each wc entry may
> > have a changelist name.
>
> Ah, I guess that's why it behaves this way now.  It's not really
> a list, but a label or tag that files may nor may not have.  Hm.

Indeed.  In fact, the nature of the changelist leads to a couple of
other behaviors which I find unintuitive as a user (though sensible
once you understand the current implementation):

As we discussed on IRC, svn commands never "recurse up" from the
highest ancestor of the files listed on the command line (or the
working directory if no files are listed).  This means that the files
referred to by any --changelist command (other than commit, which is
special) depends on the current wc.  That is, "svn diff --changelist
foo" may not show all of the changes conceptually on changelist "foo";
instead, it shows those changes that are at or below the current
directory.

(And I do mean the current directory: if you type "svn diff
--changelist foo /path/to/some/wc", the changelist will be gathered
from the *current working directory*, not from /path/to/some/wc.)

Another violation of a user's expectation that a changelist is in some
way atomic is the changelist-unsetting behavior of commit.  I like the
behavior that "svn ci --changelist bla" removes the items from the
"bla" changelist afterwards, and that you can use --keep-changelist to
keep it around.  On the other hand, I think it's rather unexpected
that "svn ci foo.c" removes foo.c from any changelist it's on even
though changelists aren't being mentioned at all.

Does anyone else find this properties of the current changelist
design/implementation surprising?

--dave

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

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

Re: If no such changelist, show we error?

Posted by Eric Gillespie <ep...@pretzelnet.org>.
"David Glasser" <gl...@davidglasser.net> writes:

> On 8/18/07, Eric Gillespie <ep...@pretzelnet.org> wrote:
> > No, if the changelist doesn't exist, I agree we should error.  I
> > was disagreeing that we should error if the cl does exist, but is
> > just empty.
> >
> > cl has committables:                commit them
> > cl has files but no committables:   silently exit 0
> > cl has no files:                    silently exit 0
> > cl does not exist:                  error out
> 
> In the current implementation, this distinction does not exist.  There
> is no mapping from changelists to entries; rather, each wc entry may
> have a changelist name.

Ah, I guess that's why it behaves this way now.  It's not really
a list, but a label or tag that files may nor may not have.  Hm.

-- 
Eric Gillespie <*> epg@pretzelnet.org

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

Re: If no such changelist, show we error?

Posted by David Glasser <gl...@davidglasser.net>.
On 8/18/07, Eric Gillespie <ep...@pretzelnet.org> wrote:
> No, if the changelist doesn't exist, I agree we should error.  I
> was disagreeing that we should error if the cl does exist, but is
> just empty.
>
> cl has committables:                commit them
> cl has files but no committables:   silently exit 0
> cl has no files:                    silently exit 0
> cl does not exist:                  error out

In the current implementation, this distinction does not exist.  There
is no mapping from changelists to entries; rather, each wc entry may
have a changelist name.

--dave

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

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

Re: If no such changelist, show we error?

Posted by Eric Gillespie <ep...@pretzelnet.org>.
"David Glasser" <gl...@davidglasser.net> writes:

> On 8/18/07, Eric Gillespie <ep...@pretzelnet.org> wrote:
> > "David Glasser" <gl...@davidglasser.net> writes:
> >
> > > certainly expect this to be a bug.  I'd hope to see either no output
> > > or a warning if you call "svn ci --changelist foo" where foo has
> > > elements but has no committables, and an error if you call "svn ci
> > > --changelist footypo" where footypo has no elements at all.
> >
> > I would treat them both as equivalent to running 'svn ci' without
> > arguments in a clean working copy, i.e. silently exit 0.
> 
> I agree on the former case (elements but no commitables).  However, if
> we stick with the current behavior (which is what you describe), we
> have this inconsistency, which I find confusing:
> 
> macbook-2:/tmp/x/w glasser$ $SVN diff
> macbook-2:/tmp/x/w glasser$ $SVN diff --changelist typo
> svn: Try 'svn help' for more info
> svn: no such changelist 'typo'
> macbook-2:/tmp/x/w glasser$ $SVN ci
> macbook-2:/tmp/x/w glasser$ $SVN ci --changelist typo
> macbook-2:/tmp/x/w glasser$

No, if the changelist doesn't exist, I agree we should error.  I
was disagreeing that we should error if the cl does exist, but is
just empty.

cl has committables:                commit them
cl has files but no committables:   silently exit 0
cl has no files:                    silently exit 0
cl does not exist:                  error out

-- 
Eric Gillespie <*> epg@pretzelnet.org

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

Re: If no such changelist, show we error?

Posted by David Glasser <gl...@davidglasser.net>.
On 8/18/07, Eric Gillespie <ep...@pretzelnet.org> wrote:
> "David Glasser" <gl...@davidglasser.net> writes:
>
> > certainly expect this to be a bug.  I'd hope to see either no output
> > or a warning if you call "svn ci --changelist foo" where foo has
> > elements but has no committables, and an error if you call "svn ci
> > --changelist footypo" where footypo has no elements at all.
>
> I would treat them both as equivalent to running 'svn ci' without
> arguments in a clean working copy, i.e. silently exit 0.

I agree on the former case (elements but no commitables).  However, if
we stick with the current behavior (which is what you describe), we
have this inconsistency, which I find confusing:

macbook-2:/tmp/x/w glasser$ $SVN diff
macbook-2:/tmp/x/w glasser$ $SVN diff --changelist typo
svn: Try 'svn help' for more info
svn: no such changelist 'typo'
macbook-2:/tmp/x/w glasser$ $SVN ci
macbook-2:/tmp/x/w glasser$ $SVN ci --changelist typo
macbook-2:/tmp/x/w glasser$

--dave

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

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

Re: If no such changelist, show we error?

Posted by Eric Gillespie <ep...@pretzelnet.org>.
"David Glasser" <gl...@davidglasser.net> writes:

> certainly expect this to be a bug.  I'd hope to see either no output
> or a warning if you call "svn ci --changelist foo" where foo has
> elements but has no committables, and an error if you call "svn ci
> --changelist footypo" where footypo has no elements at all.

I would treat them both as equivalent to running 'svn ci' without
arguments in a clean working copy, i.e. silently exit 0.

-- 
Eric Gillespie <*> epg@pretzelnet.org

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

Re: If no such changelist, show we error?

Posted by David Glasser <gl...@davidglasser.net>.
On 8/17/07, Karl Fogel <kf...@red-bean.com> wrote:
> The following IRC conversation says it all:
>
>    <kfogel> Is it a deliberate decision that 'svn commit -m logmsg
>             --changelist NON_EXISTENT_CHANGELIST_NAME' does nothing?
>             (It doesn't show any error, and it doesn't commit anything,
>             even if there are local mods, though of course those local
>             mods are not in the named changelist, since it doesn't exist.)
>
>    <kfogel> This behavior is true even if you pass --keep-changelist
>
>    <epg>    i vote bug; it should error with "no such changelist"
>
>    <kfogel> epg: I agree.
>
>    <kfogel> Hmm, I'll ask dev@, just in case there was some esoteric
>             reason why this behavior was deliberate.

None of the other --changelist commands work that way.  (They use the
svn_client_get_changelist call and explicitly check to see if the
changelist is empty.)  While commit is implemented differently, I'd
certainly expect this to be a bug.  I'd hope to see either no output
or a warning if you call "svn ci --changelist foo" where foo has
elements but has no committables, and an error if you call "svn ci
--changelist footypo" where footypo has no elements at all.

--dave

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

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