Skip to content
Snippets Groups Projects

sequoia-openpgp: fully strip unwanted optional deps

1 unresolved thread

just removing them from the features table is not enough, as cargo interprets optional deps without any feature "reverse-dependency" as "must provide automatic feature", which is the opposite of what the 'dep:' prefix is supposed to achieve.

confirmed the resulting d/control Provides list matches cargo metadata's view of the features provided by sequoia-openpgp. none of these optional deps are provided as features without the cleanup-deps patch in the first place, so nothing should depend on them either.

Signed-off-by: Fabian Grünbichler debian@fabian.gruenbichler.email

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Daniel Kahn Gillmor
    • Thanks for working on this, @fg. I noted two quibbly nit-picks (it would be nice to avoid changing the diff hunk headers unecessarily).

      i have one marginally more substantive nitpick: why comment out dependencies (with #) rather than just removing them. The patch records their removal in the Debian source, and a patch with just - lines is much clearer to read; is there a strong reason to prefer commenting them out rather than just removing them?

      More substantively, i'm not sure i fully understand the benefits for this change. I would be pretty sad if this change limited us from providing a native-rust variant of anything that depends on sequoia. At some point, it seems likely that we'll want to build architecture-independent OpenPGP verifiers at least, for example for an installer image. And having precluded the ability to build something from pure Rust would be unfortunate.

      Do you think these listed dependencies are somehow wrong upstream (in which case, is there an open upstream bug that we can refer to)? or do you think that debcargo is making some assumptions about how optional dependencies are to be interpreted, which don't match the clear upstream reality (in which case, is there an open debcargo bug that we can point to that this would be a workaround for)?

    • i have one marginally more substantive nitpick: why comment out dependencies (with #) rather than just removing them. The patch records their removal in the Debian source, and a patch with just - lines is much clearer to read; is there a strong reason to prefer commenting them out rather than just removing them?

      I just followed the style of the existing patch - I think both are used in debcargo-conf. The advantage of commenting them out is that the patched variant still shows what was removed (e.g., when browsing the crate source code in `/usr/share/cargo/registry/

      crate-
      ver` when triaging some error), but I don't mind either way, and there is no agreed-upon team standard AFAIK.

      a bit of history/background: cargo used to only have features and (possibly optional) dependencies. each optional dependency was always automatically exposed as a feature using the same name (unless an explicit feature with that name was defined). at some point cargo devs realized that this is bad for crates with lots of optional dependencies, since it's no longer clear which "features" are part of the API, and which are just a result of having optional dependencies. thus the dep: syntax was born, which tells cargo that that optional dependency should not implicitly be exposed as a feature - which allows hiding them as implementation detail and/or group related optional deps behind a single explicit feature, reducing the amount of features exposed: https://doc.rust-lang.org/cargo/reference/features.html#optional-dependencies.

      AFAICT, nothing is wrong upstream here - it's just that if we patch out all features containing dep:foo, we must also patch out the corresponding optional dependency foo - else we force cargo (and thus debcargo) into synthesizing an implicit feature foo that we don't want. hope that explains it - feel free to adapt the commit message / patch description as you see fit :smile:

      Edited by Fabian Grünbichler
    • Please register or sign in to reply
  • added 1447 commits

    Compare with previous version

  • rebased

  • Please register or sign in to reply
    Loading