If the user supplies a quilt mode option that's the same as what we
would have detected from the previous upload, we should warn them that
it's superfluous, in order to encourage people to use plain
'git debpush'.

Sean Whitton writes ("Bug#1108378: git-debpush: should warn for superfluous quilt mode option on command line"):

By "warn" I think you mean "print a message to stderr".

This sounds plausible to me as a UX improvement.

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".

Ian.

Hello,

Hmm, I think you must have misread my suggestion.
I just mean that this code:

should be run even if the user *did* supply a quilt mode, and if they
are different it's a failed check.

Hello,

if they are the _same_ we print a warning*

Sean Whitton writes ("Re: Bug#1108378: git-debpush: should warn for superfluous quilt mode option on command line"):

I don't think so.  I think I'm just being Dr Error Handling again...
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.

Ian.

Hello,

Heh.

If the user doesn't supply a quilt mode option then it's always a fatal
error if we can't find one from the history.

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?

Sean Whitton writes ("Bug#1108378: git-debpush: should warn for superfluous quilt mode option on command line"):

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.

Ian.

Hello,

Right.

Hello,

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.

This seems especially significant in light of how this program normally
does not output errors or warnings without either pausing for
confirmation or giving up completely.  Anything in its printed output
that does not involve pausing or stopping is therefore too easily missed
or ignored.

I can think of a few options:

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.

Sean Whitton writes ("Re: Bug#1108378: git-debpush: should warn for superfluous quilt mode option on command line"):

Yes.  I don't know if that's enough.

This is quite similar to:

I think this is my preference.

I think it would be better not to introduce new ways to grumble.

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.

Urgh, I don't like this, UX.

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...)

Ian.

Ian Jackson writes ("Bug#1108378: git-debpush: should warn for superfluous quilt mode option on command line"):

I mean (c).

Ian.

Hello,

Right, that is what I was thinking.  Thanks for the review.

Me neither.

Ah, yes, that's right.

We believe that the bug you reported is fixed in the latest version of
dgit, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 1108378@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Ian Jackson <ijackson@chiark.greenend.org.uk> (supplier of updated dgit package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)
Format: 1.8
Date: Sun, 10 Aug 2025 19:58:53 +0100
Source: dgit
Architecture: source
Version: 13.11+exp1
Distribution: experimental
Urgency: medium
Maintainer: Debian tag2upload Delegates <dgit-owner@debian.org>
Changed-By: Ian Jackson <ijackson@chiark.greenend.org.uk>
Closes: 1108378 1109130 1110339 1110402
Changes:
 dgit (13.11+exp1) experimental; urgency=medium
 .
   git-debpush UX improvements:
   * Always print a message before fetching from salsa.
     Closes: #1110402.  [Sean Whitton]
   * Detect debian/source/local-options, as a failed check.
     This avoids later failures.  Closes: #1110339.  [Sean Whitton]
   * Detect redundant --quilt mode option, as a failed check.
     Helps useers avoid dangerous habits.  Closes: #1108378.  [Sean Whitton]
   * Detect possible intent to use pristine-tar even if pristine-tar
     is not installed.  [Andrea Pappacoda]
 .
   tag2upload service:
   * If origs cannot be obtained from the archive (eg due to mirror delays),
     try to regenerate them.  Closes: #1109130.  (Deployment TBC.)
 .
   Documentation:
   * Workflow manpages: Replace bookworm with trixie.  [Sean Whitton]
   * Workflow manpages: Suggest sbuild -d, not sbuild -c.  [Sean Whitton]
   * tag2upload(5): Specify ! for service-critical extensions.
 .
   Minor bugfixes:
   * git-debpush: Avoid ignoring a failure of git-cat-file.  [Sean Whitton]
   * Add missing newline to overridden --force fail message.
 .
   dgit download-unfetched-origs subcommand (changes to support tag2upload):
   * Look in the source tree's current suite, specifically.
   * New --write-sha256sums subcommand option.
   * Don't download files already present in the build-products-dir.
   * Save downloaded files to .tmp names until file checksums verified.
   * Report unobtainable origs with special exit status 3.
 .
   Supporting and internal changes:
   * Cleanups and defensiveness in tag2upload-oracled.  [Sean Whitton]
   * Code cleanups, and refactoring, in git-debpush.  [Sean Whitton]
   * Much logic moved from tag2upload-obtain-origs into dgit.
 .
   Tests:
   * Refactoring and additions for testing the new functionality.
   * restore checking of error messages in tag2upload emails.
   * t2u-integration: Include the event_name field in webhook.
     (Compatibility with tag2upload-service-manager changes.)
Checksums-Sha1:
 f7223b114b00cb9049dcb24ea96114ac653ab896 2136 dgit_13.11+exp1.dsc
 918540f632ed6279e12bc9c207a50a508a462b8f 775559 dgit_13.11+exp1.tar.gz
Checksums-Sha256:
 41dc276317043bf6ef3ec4594dbf1c61ca4d8125de9f2f736fe232901165ecf6 2136 dgit_13.11+exp1.dsc
 febd3ea9409d224ddc7d5275c081990401205d28dfbed84f8860937a33ba217f 775559 dgit_13.11+exp1.tar.gz
Files:
 78a690f63fca3c3dcb0a7a9d51169ce4 2136 devel optional dgit_13.11+exp1.dsc
 7b88aff2274ae8f3253e297ab0267f95 775559 devel optional dgit_13.11+exp1.tar.gz
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEVZrkbC1rbTJl58uh4+M5I0i1DTkFAmiY9FgACgkQ4+M5I0i1
DTmexgf/XldAgj16HR6u33qezVD9eyYmTaZco2Ztdk3u/cU3APtHgbRI+/DY96Ai
2Ml8yW/fm1inpcn4iZqyjdNLL0OhgGEG7FF3GXXM7ZsgHTBo8zNXg8gdmeq0WopL
xzuNy7ksYSkkCXwL1CHqFgcQwds5Lf7Y0TNDtE3hS607lJ1ghUOEFbKADGDjK5G8
VbR+/ANtDtoFHG0aXvDq23RBet03mDnJJ1Zrowk1Qk8Yn3rLa5CXdyxb3dTxsCRj
BSFKNPhRf/Ao2hVEydPmU4BssrV2qZOEKSYeliORpJ9uafeWWQ3pPYz8rDshChjH
2rwkF21bLzhfbdyDVBYiywGjO4TkkQ==
=cvqM
-----END PGP SIGNATURE-----

We believe that the bug you reported is fixed in the latest version of
dgit, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 1108378@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Ian Jackson <ijackson@chiark.greenend.org.uk> (supplier of updated dgit package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)
Format: 1.8
Date: Sun, 10 Aug 2025 20:18:45 +0100
Source: dgit
Architecture: source
Version: 13.11
Distribution: unstable
Urgency: medium
Maintainer: Debian tag2upload Delegates <dgit-owner@debian.org>
Changed-By: Ian Jackson <ijackson@chiark.greenend.org.uk>
Closes: 1107151 1107921 1108088 1108242 1108378 1108613 1108658 1109130 1109473 1110339 1110402
Changes:
 dgit (13.11) unstable; urgency=medium
 .
   * No change upload, targeting mainline to unstable.
 .
 dgit (13.11+exp1) experimental; urgency=medium
 .
   git-debpush UX improvements:
   * Always print a message before fetching from salsa.
     Closes: #1110402.  [Sean Whitton]
   * Detect debian/source/local-options, as a failed check.
     This avoids later failures.  Closes: #1110339.  [Sean Whitton]
   * Detect redundant --quilt mode option, as a failed check.
     Helps useers avoid dangerous habits.  Closes: #1108378.  [Sean Whitton]
   * Detect possible intent to use pristine-tar even if pristine-tar
     is not installed.  [Andrea Pappacoda]
 .
   tag2upload service:
   * If origs cannot be obtained from the archive (eg due to mirror delays),
     try to regenerate them.  Closes: #1109130.  (Deployment TBC.)
 .
   Documentation:
   * Workflow manpages: Replace bookworm with trixie.  [Sean Whitton]
   * Workflow manpages: Suggest sbuild -d, not sbuild -c.  [Sean Whitton]
   * tag2upload(5): Specify ! for service-critical extensions.
 .
   Minor bugfixes:
   * git-debpush: Avoid ignoring a failure of git-cat-file.  [Sean Whitton]
   * Add missing newline to overridden --force fail message.
 .
   dgit download-unfetched-origs subcommand (changes to support tag2upload):
   * Look in the source tree's current suite, specifically.
   * New --write-sha256sums subcommand option.
   * Don't download files already present in the build-products-dir.
   * Save downloaded files to .tmp names until file checksums verified.
   * Report unobtainable origs with special exit status 3.
 .
   Supporting and internal changes:
   * Cleanups and defensiveness in tag2upload-oracled.  [Sean Whitton]
   * Code cleanups, and refactoring, in git-debpush.  [Sean Whitton]
   * Much logic moved from tag2upload-obtain-origs into dgit.
 .
   Tests:
   * Refactoring and additions for testing the new functionality.
   * restore checking of error messages in tag2upload emails.
   * t2u-integration: Include the event_name field in webhook.
     (Compatibility with tag2upload-service-manager changes.)
 .
 dgit (13.10) experimental; urgency=medium
 .
   git-debpush user experience improvements:
   * Print some status messages while processing.
   * Better error messages when git-deborig tag search fails.
   * New --help, -h options.  Closes: #1109473.
 .
   tag2upload infrastructure:
   * Switch to using new tag2upload-obtain-origs script.
     Closes: #1108658.  [Ian Jackson]
   * tag2upload Manager protocol: Introduce some protocol state names.
     [Ian Jackson]
   * tag2upload-oracled: Speak Manager protocol version 5.
   * tag2upload-builder-rebuild: Request restarting workers.
 .
   git-debpush miscellaneous:
   * Expand and update statement of design principles.
   * Delete a completed TODO.
 .
 dgit (13.9) experimental; urgency=medium
 .
   tag2upload infrastructure:
   * Improve and shorten command lines appearing in report emails.
   * Check upstream tag/commit correspondence even when using existing origs.
   * Minor performance improvement for native packages (skip pre-fetch).
 .
   Internal changes:
   * Testing improvements for tag2upload.
   * changelog: Start 13.9
   * Introduce new tag2upload-obtain-origs script.
 .
 dgit (13.8) experimental; urgency=medium
 .
   git-debpush user experience improvements:
   * Check for and handle an existing tag for this version.
   * Try to check our branch against the remote target branch,
     to detect releasing from behind mainline, and avoid push failures.
     Closes: #1107921.
 .
   Internal changes:
   * dgit: Provide abbreviated --t2u-* options, to support log improvements.
   * git-debpush: Improve comment about remote branch.  [Sean Whitton]
   * git-debpush: Coding style cleanups.
   * dir-locals.el: Set fill-column to 78 for all languages.
 .
 dgit (13.7) experimental; urgency=medium
 .
   * Demo upload for talk.
 .
 dgit (13.6) experimental; urgency=medium
 .
   * Demo upload for talk rehearsal.
 .
 dgit (13.5) experimental; urgency=medium
 .
   git-debpush user experience improvements:
   * Check for tag already existing at the remote.  Part of #1107921.
   * Improve error message for tag already existing locally.
   * Check for uncommitted changes in working tree.
     Closes: #1108088.  [Sean Whitton]
 .
   tag2upload service:
   * Properly RFC2047-escape non-ASCII names.  Closes: #1108613.
   * Hide virt runes in a per-invocation ssh wrapper  Closes: #1107151.
   * Print oracle and builder dgit versions in transcript.  Closes: #1108242.
   * d-r-s t2u: Add missing -oBatchMode=yes option to ssh invocation.
 .
   CI and administrivia:
   * CI: Check for missing Signed-off-by.
   * git-debpush: Add missing copyright notice.  [Sean Whitton]
   * Add missing Closes for #1108180 to changelog entry for 12.125.
   * CI: Drop compatibility tests for buster, which is gone from the archive.
 .
   Suppporting changes:
   * git-debpush: Figure out the previous maintainer upload version.
   * dgit: Honour DGIT_VERSION in dgit --version and use it in ./using-these.
   * git-debpush: Reorganisation and tidying.  [Ian Jackson and Sean Whitton]
 .
 dgit (13.4) experimental; urgency=medium
 .
   Documentation:
   * t2u o2m: Describe version 5, with 'restart' message.  [Sean Whitton]
   * t2u FAQ: Clarify that debian/-only repos still need
     upstream source in git.  [Christoph Berg]
   * t2u FAQ: Remove much obsolete material.
 .
   Tests:
   * t2u: Test "tag changed between fetches" path.
   * CI: Check for blocking todos (three or more X's).
 .
   Internals:
   * Avoid blocking-todo markers in a few places in docs and comments.
   * dgit-repos-server: Fix a comment about indistrodir.
   * tests: debpolicy-taintrm: Downgrade a TODO.
Checksums-Sha1:
 9193e59edd3b0c18c8347e09d776d313b66a0eb0 2111 dgit_13.11.dsc
 7f99505fd4f2ce48b05d2c284027558ee183d56b 775411 dgit_13.11.tar.gz
Checksums-Sha256:
 4824f3e9a499a6c7e0699c6f7792f16b0d1105a86bac64644a70b73cf0cacf00 2111 dgit_13.11.dsc
 a1fca8a024897385e6238b0ceef172d4754493071d51ac0def18b17b4ab68bb6 775411 dgit_13.11.tar.gz
Files:
 7966448f18a47e620270ba6888e153d6 2111 devel optional dgit_13.11.dsc
 51e56d2f6a4773ce3cc47156c6e9c0bc 775411 devel optional dgit_13.11.tar.gz
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEVZrkbC1rbTJl58uh4+M5I0i1DTkFAmiZDtYACgkQ4+M5I0i1
DTmhdQf/VUwTc+1xg3POYk6Nn7ai83HGt+Gc36FCeQLp8qjNgQ3lCtS2pcsVBcmE
VS9/Feh6q5o9jpbMob/TBKaH6VdyNs0koEEi2VDC9ELi7KVZkPgQlsiq31IAMN6B
00gQ9ZJzs0A5Gfy0dAFUXBNE8EGHufhq3Ee87aFmvgLSipo8r6I+GJC1u+5cBoSr
TV7SsBAwyNjy+nIYbg3rCU2cYjI8vSGNySrwnXTyKrJ8HnCY9nC+DGSBrgWWvqg3
Sj58Ny3MIqkoH6d/bapwpawcdK5kowv3ToDvUhSH1Ix4nQCCpvTRcdOlp2EnHVrZ
3OVU4lUcKy3AKooeP1QU8uaAd1RYGw==
=du5z
-----END PGP SIGNATURE-----