[MERGE] Fix for 306394: warning, not error, on non-existent file.

Aaron Bentley aaron at aaronbentley.com
Fri Dec 26 16:34:33 GMT 2008


Stephen J. Turnbull wrote:
> Aaron Bentley writes:
>  > It could presumably request a full status of the tree instead,
>  > which would be more efficient than asking about invividual files.
> 
> vc.el deals with many different VCSes, and asking about the whole
> tree might or might not be more efficient.

But one of those VCSes is Bazaar, and if you're writing a backend for
Bazaar, it would make sense to use it in such a way that it gives the
best results you can get.

> In particular, it might
> provide excessively verbose output which then would have to have the
> relevant bits parsed out.  In any case, that's not what it does
> ... since, oh, 1992 or so.

But Bazaar didn't exist in 1992.  I can only assume someone wrote a
backend for Bazaar much more recently.

>  > But from my POV, that's not what status is for.  It's for asking about
>  > files that are versioned, or might be versioned.
> 
> What makes you assume that the (at this instant) non-existent file
> wasn't versioned in the past, or isn't scheduled for versioning in the
> future?

It is impossible for Bazaar to schedule a file to be added if the file
does not exist.  But yes, it's reasonable to look up the status of a
recently-deleted file.

> Isn't it possible that vc.el, which (in this scenario) has a
> much more intimate relationship to the user, knows far more about
> the user's plans than bzr does?

It is possible, but then again, it's possible that bzr knows much more
about its internal state than vc.el.

>  > I expect there are two common usages for "status":
>  > - "bzr status"
>  > - "bzr status $FILE"
> 
> Why are you surprised that surprising things happen?<wink>  When bzr
> is being driven by a program, what is common usage is determined by
> the vagaries of the implementation, not by what humans might (or might
> not) reasonably expect.

I think it's pretty obvious that I'm talking about bzr being driven by a
human.  But I would also argue that being driven by vc.el is *not* the
common case.

>  > For "bzr status $FILE", it might as well be an error.  It's only when
>  > you're listing multiple files that it really makes a difference.  And
>  > listing multiple files at the commandline is usually not worth the effort.
> 
> There's no effort involved here because vc.el is doing the work.

Again, this rejoinder doesn't make sense in the context of human-driven
usage.

> In any case, I disagree with your assessment; "$VCS status *.$EXT" is
> a common idiom for me.

Fair enough.  But in this scenario, the shell ensures the files exist
(unless none of them do), so we cannot encounter a situation where some
files exist and others do not.  So it does not inform a discussion of
how to behave when some files exist and others do not.

So I would continue to argue that there is no technical argument has
been raised that makes this a blocker.  I can see that it might be
awkward to work around, of course, but it does not seem impossible.

I should also point out that the recommended way of interacting with
Bazaar from a program is the xmloutput plugin, so that would be a much
better place for these kinds of changes.

>  > > For some of those names, the status is "does not exist".  Fine --
>  > > let's report that, and report the status of the other paths as
>  > > well, rather than treat non-existence as so special that it must
>  > > suppress other statuses from being reported.
>  > 
>  > If you really want to treat non-existence as a status, it does not make
>  > sense to emit a warning.  Warnings go to stderr, while normal status
>  > output goes to stdout.  Using a warning would mean that you cannot
>  > capture the full status by redirecting stdout.  But if you're going to
>  > emit to stdout, the output should conform to our existing output styles.
> 
> ehem ... if your quote is any fair representation of KarlFogelThought,
> that is exactly what he is suggesting.

I'm glad my critique matches your interpretation of that Karl Fogel
quote, because it also matches mine.

The two parts of my critique are:
1. If non-existence is a status, it should go to stdout, instead of
being a warning and going to stderr.
2. If non-existence goes to stdout, it should match our existing output
styles.

However, in the supplied patch, Karl invokes trace.warning, so it fails
1.  He stringifies a PathsDoNotExist, but this does not match either our
default status output or our short status output.  So it fails 2.

So as you can see, his proposed patch does not follow my critique.  You
seem to agree that my critique is consistent with his argument,
therefore his patch is inconsistent with his argument.  That is the
point I was trying to make.

Aaron



More information about the bazaar mailing list