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

Parse target_abi if it's present even though it's unstable #52

Closed

Conversation

carols10cents
Copy link
Contributor

@carols10cents carols10cents commented Nov 4, 2022

Hi!! 👋🏻

Checklist

  • I have read the Contributor Guide
  • I have read and agree to the Code of Conduct
  • I have added a description of my changes and why I'd like them included in the section below

Description of Changes

This supports parsing the currently unstable target_abi cfg predicate, which is described in:

Because this is unstable, the Abi will be None for all current builtins.

However, this change enables cfg-expr to successfully parse cfg predicates that contain target_abi for those crates that decide to use it.

I believe once target_abi is stabilized that this code will Just Work™️, but of course I don't have a way to predict the future 😅 I did hack the update binary to use nightly Rust and it did set the Abi field as I expected, so 🤞🏻

The reason I want this is because:

Error: 
   0: building package graph failed
   1: failed to construct package graph: for package 'ahash 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)': for dependency 'getrandom', parsing target 'cfg(any(target_arch = "wasm32", target_abi = "unknown"))' failed: invalid cfg() expression

So from what I can tell, we either can't upgrade ahash or we can't use cargo hakari without this change to cfg-expr.

I'm totally open to being told no, this isn't something you want to support, and I'll figure out a workaround some other way :) But this seemed like the "right" place to fix it.

Related Issues

None directly related that I could find; I searched for "target_abi" and "nightly". Support older builtins is tangentially related as this is supporting all future possible predicates whereas that issue expresses desire to support all past possible ones.

There are other unstable cfg attributes like target_has_atomic_equal_alignment that ideally should be supported here too, and I'm happy to work on adding those if you'd rather support all unstable cfgs or none of them, but target_abi is the only one that's affecting me directly right now 😉

Thank you!! ❤️

@Jake-Shadle
Copy link
Member

I believe if you rebase from main most of the lints will be fixed.

@carols10cents carols10cents deleted the support-target-abi branch November 7, 2022 17:08
carols10cents added a commit to integer32llc/guppy that referenced this pull request Nov 7, 2022
To get support for parsing target_abi. See:

<EmbarkStudios/cfg-expr#52>
<EmbarkStudios/cfg-expr#54>

This gets the new target_abi test to pass.
carols10cents added a commit to integer32llc/guppy that referenced this pull request Nov 7, 2022
To get support for parsing target_abi. See:

<EmbarkStudios/cfg-expr#52>
<EmbarkStudios/cfg-expr#54>

This gets the new target_abi test to pass.
sunshowers pushed a commit to sunshowers/guppy that referenced this pull request Nov 7, 2022
To get support for parsing target_abi. See:

<EmbarkStudios/cfg-expr#52>
<EmbarkStudios/cfg-expr#54>

This gets the new target_abi test to pass.
sunshowers pushed a commit to sunshowers/guppy that referenced this pull request Nov 7, 2022
To get support for parsing target_abi. See:

<EmbarkStudios/cfg-expr#52>
<EmbarkStudios/cfg-expr#54>

This gets the new target_abi test to pass.
sunshowers pushed a commit to guppy-rs/guppy that referenced this pull request Nov 7, 2022
To get support for parsing target_abi. See:

<EmbarkStudios/cfg-expr#52>
<EmbarkStudios/cfg-expr#54>

This gets the new target_abi test to pass.
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.

2 participants