Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Windows builds in CI #250

Merged
merged 3 commits into from
Sep 16, 2024
Merged

Fix Windows builds in CI #250

merged 3 commits into from
Sep 16, 2024

Conversation

zoedberg
Copy link
Contributor

This will fix the windows build on the CI by adding nasm as a dependency

Copy link

codecov bot commented Sep 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 3.0%. Comparing base (56f8d4c) to head (8ceff54).
Report is 4 commits behind head on master.

Additional details and impacted files
@@          Coverage Diff          @@
##           master   #250   +/-   ##
=====================================
  Coverage     3.0%   3.0%           
=====================================
  Files          14     14           
  Lines        1753   1753           
=====================================
  Hits           53     53           
  Misses       1700   1700           
Flag Coverage Δ
rust 3.0% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zoedberg
Copy link
Contributor Author

I'm looking for a solution for the wasm-testing job, please don't merge this in the meantime

@zoedberg
Copy link
Contributor Author

I've opened BP-WG/bp-esplora-client#8 and add a commit here to fix the wasm build (I'll change the git dependency after the PR gets merged or I'll remove it if you release a new version of bp-esplora-client). Unfortunately there's no way to conditionally exclude dependencies based on target, so I had to add a dedicated feature for wasm. Moreover wasm-pack doesn't seem to provide a way to select features (it uses the default ones), so I had to remove all default features from here. CI is still failing because of some missing feature gates (unnoticed because of the default features), I will fix this tomorrow or in the following days

@dr-orlovsky
Copy link
Member

there's no way to conditionally exclude dependencies based on target

There is:

[target.'cfg(target_arch = "wasm32")'.dependencies]
# your dependencies

@zoedberg
Copy link
Contributor Author

Sorry, I meant features, not dependencies. I've found this related open issue rust-lang/cargo#1197 so it seems it's not possible at the moment

@zoedberg
Copy link
Contributor Author

@dr-orlovsky all CI jobs pass, IMO this can be merged

Copy link
Member

@dr-orlovsky dr-orlovsky left a comment

Choose a reason for hiding this comment

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

ACK 8ceff54

@dr-orlovsky dr-orlovsky changed the title fix CI Fix Windows builds in CI Sep 16, 2024
@dr-orlovsky dr-orlovsky added the ci Continious integration-related issues label Sep 16, 2024
@dr-orlovsky dr-orlovsky merged commit e3bb8e3 into RGB-WG:master Sep 16, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continious integration-related issues
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants