Ticket #43972

generate_packets.py: Make output paths for each file optional

Date d'ouverture: 2022-02-22 23:04 Dernière mise à jour: 2022-02-26 22:16

Rapporteur:
Propriétaire:
Type:
État:
Atteints
Composant:
Jalon:
Priorité:
5 - moyen
Sévérité:
5 - moyen
Résolution:
Fixed
Fichier:
1

Détails

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 History (3/5 Histories)

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

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 Updated by: alienvalkyrie
  • Propriétaire Update from (Aucun) to alienvalkyrie
  • Résolution Update from Aucun to Accepted
Commentaire

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 Updated by: alienvalkyrie
  • État Update from Ouvert to Atteints
  • Résolution Update from Accepted to Fixed

Modifier

Please login to add comment to this ticket » Connexion