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

remove portable flag #4646

Merged
merged 2 commits into from
Apr 8, 2024
Merged

remove portable flag #4646

merged 2 commits into from
Apr 8, 2024

Conversation

wileyj
Copy link
Contributor

@wileyj wileyj commented Apr 5, 2024

ref: #4638

the options in .cargo/config are not being applied, this PR removes them entirely and adds some text in the README on how to enable building for native-cpu.

removing the portable flag will have some downstream effects to the release build process that we'll need to address once merged (basically removing the portable feature flag, and re-enabling macos-arm64 stacks-signer binary builds).

@wileyj wileyj self-assigned this Apr 5, 2024
@wileyj wileyj added the CI label Apr 5, 2024
Copy link

codecov bot commented Apr 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.20%. Comparing base (8d4da57) to head (e28fd65).

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #4646      +/-   ##
==========================================
- Coverage   83.37%   83.20%   -0.17%     
==========================================
  Files         470      470              
  Lines      332981   332981              
  Branches      317      317              
==========================================
- Hits       277627   277068     -559     
- Misses      55346    55905     +559     
  Partials        8        8              

see 51 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d4da57...e28fd65. Read the comment docs.

Copy link
Contributor

@obycode obycode left a comment

Choose a reason for hiding this comment

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

This is strange that this doesn't work, but the reversal looks good to me.

@wileyj
Copy link
Contributor Author

wileyj commented Apr 5, 2024

This is strange that this doesn't work, but the reversal looks good to me.

it's mentioned in the ref ticket #4638 - it was silently being ignored and the docs aren't very up front about it.
apparently setting rustflags via that method is not supported in cargo (thanks to @zone117x pointing that out)

#4638 (comment)

@wileyj wileyj enabled auto-merge April 6, 2024 17:48
@wileyj wileyj requested a review from a team April 8, 2024 18:48
Copy link
Member

@kantai kantai left a comment

Choose a reason for hiding this comment

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

This LGTM, but do you want this in develop rather than next?

@wileyj wileyj added this pull request to the merge queue Apr 8, 2024
@wileyj
Copy link
Contributor Author

wileyj commented Apr 8, 2024

This LGTM, but do you want this in develop rather than next?

good catch - i think it should be fine, since this is mostly needed for release builds (and should make it's way to develop soon anyways)

Merged via the queue into next with commit 844a2a7 Apr 8, 2024
2 checks passed
wileyj added a commit to stacks-network/actions that referenced this pull request Apr 8, 2024
@wileyj wileyj mentioned this pull request Apr 9, 2024
wileyj added a commit to wileyj/actions that referenced this pull request Jul 10, 2024
* upstream/main: (26 commits)
  link on summary the documentation on how to fix the mutants' output
  mutants: add link to summary on failed output job cases
  documentation how mutants output should be treated
  fix: run one test at a time
  fix(rustfmt): Exit script with error code on failure
  fix(rustfmt): Read `rustfmt` alias from `.cargo/config` or `.cargo/config.toml`
  feat: remove `./`
  feat: check if release dir exists before trying to access it
  feat: setup rustfmt in composite actions
  feat: get rid of the `release/` path in the generated checksums
  fix: remove leftover git diff file
  feat: upload and download artifacts under different names
  feat: update all actions versions used
  more whitepsace removal
  remove whitespace
  remove portable build flag
  remove portable stacks-network/stacks-core#4646
  change to check for unviable from inexistent file to empty file
  remove conditional build
  build all binaries and copy them with find where possible (macos arm64 is outlier)
  ...
@blockstack-devops
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@stacks-network stacks-network locked as resolved and limited conversation to collaborators Nov 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants