Ticket #43972

generate_packets.py: Make output paths for each file optional

Eröffnet am: 2022-02-22 23:04 Letztes Update: 2022-02-26 22:16

Auswertung:
Verantwortlicher:
Typ:
Status:
Geschlossen
Komponente:
Meilenstein:
Priorität:
5 - Mittel
Schweregrad:
5 - Mittel
Lösung:
Gefixt
Datei:
1

Details

Part of #43927. Followup to #43971. Sometimes (like current CI meson build) it might be desirable to only write some, not all, of the output files. This should be officially supported by making the respective path arguments optional, rather than by passing empty paths.

Ticket-Verlauf (3/5 Historien)

2022-02-22 23:04 Aktualisiert von: alienvalkyrie
  • New Ticket "generate_packets.py: Make output paths for each file optional" created
2022-02-24 07:31 Aktualisiert von: alienvalkyrie
Kommentar

Doing this properly is nontrivial. Just making the positional arguments optional would mean it's only possible to omit the later arguments, not the earlier ones. We need to add --name options for each path.

There are two ways we could go about this:

  • The ugly, complicated, but backwards-compatible way, which is to allow both positional and named arguments; doing this in a way that reliably works, produces understandable help text (without having to manually write it) and understandable error messages will probably be somewhat convoluted.
  • The quick, clean, but not backwards-compatible way, which is to remove the positional arguments and completely replace them with named arguments; this would also require changing the relevant calls in the build system (meson.build and common/Makefile.am, as far as git grep can tell), and it would break anyone else's custom builds. (Though, given the helpful usage instructions printed by argparse, most people should be able to easily fix their builds.)

(I assume one can tell from my choice of words which one I'd prefer, but to spell it out – unless we desperately need backwards compatibility here, I'd go with the second option.)

There's also the question of whether we'd still allow passing empty strings as an alternative to omitting arguments; IMO that decision should mirror the first one (i.e. keep allowing it only if we choose to maintain backwards compatibility).

2022-02-25 08:13 Aktualisiert von: alienvalkyrie
  • Verantwortlicher Update from (Keine) to alienvalkyrie
  • Lösung Update from Keine to Accepted
Kommentar

Attached patch takes the second option, i.e. breaks backwards compatibility for potential custom builds. If there is any reason why we need to maintain backwards compatibility here, now would be a good time to mention it.

2022-02-26 22:16 Aktualisiert von: alienvalkyrie
  • Status Update from Offen to Geschlossen
  • Lösung Update from Accepted to Gefixt

Bearbeiten

Please login to add comment to this ticket » Anmelden