But implementing this may be nontrivial. quilt mode archaeology is
complex, and currently it is allowed to crash whenever it can't figure
things out. I think this proposal involves distinguishing "something
is unexpected about the git structure which means that we cannot
determine the quilt mode automatically" from "something is seriously
wrong and we should crash even if the user specified --quilt".
# If the user didn't supply a quilt mode, look for it in a previous
# tag made by this script
if [ "x$quilt_mode" = x ] && [ "$format" = "3.0 (quilt)" ]; then
set +o pipefail # perl will SIGPIPE git-cat-file(1) here
if [ "x$last_debian_tag" != x ]; then
quilt_mode=$(git cat-file -p $(git rev-parse "$last_debian_tag") \
| perl -wne'/^\[dgit.*--quilt=([a-z+]+).*\]$/
or next;
print "$1\n"; exit')
fi
set -o pipefail
fi
should be run even if the user *did* supply a quilt mode, and if they
are different it's a failed check.
On Tue 01 Jul 2025 at 02:46pm +01, Ian Jackson wrote:
But implementing this may be nontrivial. quilt mode archaeology is complex, and currently it is allowed to crash whenever it can't figure things out. I think this proposal involves distinguishing "something
is unexpected about the git structure which means that we cannot
determine the quilt mode automatically" from "something is seriously
wrong and we should crash even if the user specified --quilt".
Hmm, I think you must have misread my suggestion.
I just mean that this code:
# If the user didn't supply a quilt mode, look for it in a previous
# tag made by this script
if [ "x$quilt_mode" = x ] && [ "$format" = "3.0 (quilt)" ]; then
set +o pipefail # perl will SIGPIPE git-cat-file(1) here
if [ "x$last_debian_tag" != x ]; then
quilt_mode=$(git cat-file -p $(git rev-parse "$last_debian_tag") \
| perl -wne'/^\[dgit.*--quilt=([a-z+]+).*\]$/
or next;
print "$1\n"; exit')
fi
set -o pipefail
fi
should be run even if the user *did* supply a quilt mode, and if they
are different it's a failed check.
Your point is that if they do supply one, then we need to distinguish
between different possible failures to find one from the history,
between those that should still be fatal errors and those which should
be ignored because the --quilt option the user has passed is pseudo-overriding them?
I don't think so. I think I'm just being Dr Error Handling again...
This code can fail due to (for example) the tag named by
last_debian_tag not existing, or having weird syntax.
In that case, if the user supplied --quilt=, that's silently fine: we
failed to check the user's work, but the anomaly is presumably why the
user is passing that option.
To put it in type threory terms in Rust syntax.
Before:
fn determine_quilt_mode_automatically(...)
-> Result<QuiltMode, FatalError>
After:
fn determine_quilt_mode_automatically(...)
-> Rdsult<Result<QuiltMode, CouldNotDetermineQuiltMode>, FatalError>
This means that every program call needs to be considered for whether
failure of this program (or perhpas, *this kind* of failure of this
program) is "expected" (-> CouldNotDetermineQuiltMode, might be
silently ignored) or "unexpedcted" (-> FatalError, always crash).
That is doable but it's not trivial.
Sean Whitton writes ("Bug#1108378: git-debpush: should warn for superfluous quilt mode option on command line"):
Your point is that if they do supply one, then we need to distinguish
between different possible failures to find one from the history,
between those that should still be fatal errors and those which should
be ignored because the --quilt option the user has passed is
pseudo-overriding them?
Yes.
But, some nuance:
I'm not sure "pseudo-overriding" is the right framing. When the user
passes --quilt we're only doing all this work *only* to detect user
mistake (so that we can fail a check, or print a warning).
Detecting user mistakes is a best effort activity for git-debpush.
So being unable to determine whether this kind of mistake has been
made is not an error in itself. But, while we're doing that, if we
discover that everything is totally broken, we *should* bomb out.
I think that we might want to do more than just print a warning to
stderr, because that might not help users enough to learn how
git-debpush works or to change their habits.
1. say that the quilt mode option was superfluous, then print
[acknowledge] (or similar), and require the user to press return
before continuing
2. make it work just like a failed check. The user has to type y and
then return to continue. Sounds a bit strange in the abstract but
might be the most useful in practice.
3. just exit, requiring the user to run the command again without the
superfluous quilt option.
For all three, --batch would switch to just a printed warning.
Reasonable options are:
a. Refuse
b. Failed check (prompt)
c. Grumble on stderr but do it anyway
d. Silently do it anyway
Currently we don't have any (b). I'm fine if we don't add them now,
but of course programs we call (eg git) might well do (b) so we can't
avoid them entirely.
Sean Whitton writes ("Re: Bug#1108378: git-debpush: should warn for superfluous quilt mode option on command line"):
2. make it work just like a failed check. The user has to type y and
then return to continue. Sounds a bit strange in the abstract but
might be the most useful in practice.
I think this is my preference.
I think it would be better not to introduce new ways to grumble.
3. just exit, requiring the user to run the command again without the
superfluous quilt option.
Urgh, I don't like this, UX.
For all three, --batch would switch to just a printed warning.
I think --batch turns failed checks into errors? That seems right
here. If you're writing script and know you're passing a possibly-superfluous quilt mode, you're writing a script which unconditionally overrides git-debpush's determination, so you ought to
pass a specific --force-failed-check option (I forget how those are spelled...)
| Sysop: | Keyop |
|---|---|
| Location: | Huddersfield, West Yorkshire, UK |
| Users: | 715 |
| Nodes: | 16 (3 / 13) |
| Uptime: | 46:32:36 |
| Calls: | 12,112 |
| Calls today: | 3 |
| Files: | 15,010 |
| Messages: | 6,518,487 |