Skip to content

Conversation

@Mshehu5
Copy link
Contributor

@Mshehu5 Mshehu5 commented Dec 11, 2025

This PR addresses the following changes:

  • Removes the .udl filter from Nix cause ffi layer doesn't use udl files anymore as discussed in the mob review meeting.
  • Adds a Machete dependency check to the Nix flake, as referenced in issue nix flake tracking issue #457
  • Removes dependencies flagged as unused by Machete

Note: I kept getrandom because the wasm_js feature requires getrandom/js so it’s an actual dependency.
If you remove it and run cargo check you’ll see this.

After adding machete check I ran nix flake check which displayed the log below

❌ checks.x86_64-linux.payjoin-workspace-machete
error: Cannot build '/nix/store/qfx7v13m73iq8v4b0izi7260xdd5hx15-payjoin-workspace-machete-no-version.drv'.
       Reason: builder failed with exit code 1.
       Output paths:
         /nix/store/lwvvwfbhnfqkmh1rzp46l9rv8l33s4yv-payjoin-workspace-machete-no-version
       Last 25 log lines:
       >        futures
       >         hyper-rustls
       >    rcgen
       >   sled
       > payjoin-test-utils -- ./payjoin-test-utils/Cargo.toml:
       >     log
       > payjoin-ffi -- ./payjoin-ffi/Cargo.toml:
       >    base64
       >  getrandom
       >       hex
       > payjoin-ffi-wrapper -- ./payjoin-ffi/dart/native/Cargo.toml:
       >        home
       >
       > If you believe cargo-machete has detected an unused dependency incorrectly,
       > you can add the dependency to the list of dependencies to ignore in the
       > `[package.metadata.cargo-machete]` section of the appropriate Cargo.toml.
       > For example:
       >
       > [package.metadata.cargo-machete]
       > ignored = ["prost"]
       >
       > You can also try running it with the `--with-metadata` flag for better accuracy,
       > though this may modify your Cargo.lock files.
       >
       > Done!

Second time running after removing dependencies I got the error below. It did not catch this in the first check :

❌ checks.x86_64-linux.payjoin-workspace-machete
error: Cannot build '/nix/store/ymsycgba5z2vw53x3ibrf081fwys464a-payjoin-workspace-machete-no-version.drv'.
       Reason: builder failed with exit code 1.
       Output paths:
         /nix/store/lz9a60g9wfkjrmmyjn164z7n1sr1sck2-payjoin-workspace-machete-no-version
       Last 25 log lines:
       > Running phase: configurePhase
       > will append /build/source/.cargo-home/config.toml with contents of /nix/store/whmqvxdpvzjqzzjb38djwznb5fm9qv63-vendor-cargo-deps/config.toml
       > default configurePhase, nothing to do
       > Running phase: buildPhase
       > +++ command cargo --version
       > cargo 1.93.0-nightly (bd979347d 2025-12-02)
       > Running phase: checkPhase
       > +++ command cargo machete
       > Analyzing dependencies of crates in this directory...
       > cargo-machete found the following unused dependencies in this directory:
       > payjoin-directory -- ./payjoin-directory/Cargo.toml:
       >     hyper-rustls
       >
Pull Request Checklist

Please confirm the following before requesting review:

drop the .udl match from the cleanSourceWith filter
as Uniffi does not use .udl files anymore
enforces unused dependency detection in nix flake check
@coveralls
Copy link
Collaborator

coveralls commented Dec 11, 2025

Pull Request Test Coverage Report for Build 20144561401

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 83.176%

Totals Coverage Status
Change from base Build 20141275772: 0.0%
Covered Lines: 9665
Relevant Lines: 11620

💛 - Coveralls

@Mshehu5 Mshehu5 marked this pull request as ready for review December 11, 2025 12:03
uniffi = { version = "0.30.0" }
url = "2.5.4"

[package.metadata.cargo-machete]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain why this is necessary here instead of a removal?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I kept getrandom because the wasm_js feature requires getrandom/js so it’s an actual dependency.
If you remove it and run cargo check you’ll see this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this exclusion should be explained in a code comment here


[dependencies]
base64 = "0.22.1"

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra line

Copy link
Collaborator

@spacebear21 spacebear21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I have some very minor comments but otherwise lgtm

Dropped unused dependency flagged by cargo machete across codebase. Kept getrandom because the wasm_js feature requires getrandom/js and marked it ignored in machete metadata
Copy link
Collaborator

@benalleng benalleng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack fe4858c

@benalleng benalleng merged commit 2107bdc into payjoin:master Dec 11, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants