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

Release 0.3? #433

Closed
josephlr opened this issue May 26, 2024 · 48 comments · Fixed by #563
Closed

Release 0.3? #433

josephlr opened this issue May 26, 2024 · 48 comments · Fixed by #563
Milestone

Comments

@josephlr
Copy link
Member

We have a few changes coming up (#425, #414 , #432) which will make the following changes to the compatibility of this crate:

  • Increase the Minimum Supported Rust Version (MSRV) from 1.36 (released July 4, 2019) to 1.56 (released October 21, 2021).
  • Increase the supported OS version of *-pc-windows-{gnu,msvc} from Windows Vista (released November 30, 2006) to Windows 10 (released July 15, 2015).
    • Rust increased its minimum Windows OS version from Windows 7 to Windows 10 in February 26th, 2024.
    • Older versions of Windows are still supported under the x86_64-win7-windows-msvc and i686-win7-windows-msvc targets
    • We would intend to continue to support the *-win7-* targets (via RtlGenRandom)
  • We are not making any changes to the API of this crate
  • We are not making any changes to the features of this crate

The Rust Semver Compatibility Guide does not specify if such actions would be breaking changes. Both:

are listed as "Possibly-breaking" changes.

My personal preference would be to release these changes as 0.2.16 (not consider this a breaking change) because:

  • getrandom is widely used throughout the Rust ecosystem so updating all callers can be somewhat tedious.
  • It's not clear how a user can have a custom implementation work with both versions of getrandom.
  • Both the MSRV increases and the Windows platform increases are less disruptive than an Semver-breaking change.

However, if we do decide to have this be a breaking change, we should discuss if there are any other semver-breaking changes we wish to make (see #346 and #230). Another option would just be to get consensus on #232 and release these changes as 1.0

@josephlr
Copy link
Member Author

CC: @dhardy @vks for how this might affect rand

@newpavlov
Copy link
Member

newpavlov commented May 27, 2024

We are not making any changes to the API of this crate

There was a proposal to rename getrandom(_uninit) to fill(_uninit), plus there is #365, but I do not have a concrete proposal for it now. Associated Error constants could also use a bit of work, e.g. I am not sure it's worth to expose constants like VXWORKS_RAND_SECURE on all platforms. We also can use this release to reconsider our cfgs (e.g. #230 and #346). It's also worth to tweak __getrandom_custom's signature to remove this hack.

Moreover, we can use this release to relax our MSRV policy in the vein suggested by @briansmith, but this should be done together with rand.

getrandom is widely used throughout the Rust ecosystem so updating all callers can be somewhat tedious.

I think it's rarely used directly, usually it's done through rand crates.

It's not clear how a user can have a custom implementation work with both versions of getrandom.

It's possible to do this:

[dependencies]
getrandom02 = { version = "0.2", package = "getrandom" }
getrandom = "0.3"

And call the registering macros from each crate (assuming extern functions are called differently).

@dhardy
Copy link
Member

dhardy commented May 28, 2024

As @josephlr says, pushing these changes to a 0.2.x release does not break any formal stability guarantees and likely won't cause a large number of issues, though it's impossible to be certain.

I have a slight preference for bumping the version to 0.3 since the drawbacks are small:

  • Custom backends can support multiple versions of getrandom as @newpavlov says. An example of this in action: winit supports three versions of raw-window-handle (optionally).
  • No other dependants will be affected. Some build trees may end up using multiple getrandom versions, but this hardly matters.

@briansmith
Copy link
Contributor

  • No other dependants will be affected. Some build trees may end up using multiple getrandom versions, but this hardly matters.

I understand what you're saying, but in practice it does matter. Many people have their CI set up to reject multiple versions of a dependency by default. so they have to either make an exception to their CI rules, and/or they submit PRs upstream to try to upgrade everybody to the new version, which is extra work for everybody. Then those upstream projects (such as some I maintain) end up having to choose to drop support for 0.2 to help these people, or spend time trying to explain to them (over and over, indefinitely) why it "hardly matters." Either way it ends up being disproportionate amount of effort.

If we release 0.3:

  • What will we cap the MSRV to for the 0.3 series? 1.63 and insist on a 0.4 for any higher MSRV? Otherwise, I guess many projects will want to stick with 0.2.
  • When we add new OS support for 0.3, will we backport those to 0.2? In general will we keep maintaining 0.2? Depending on the MSRV policy for 0.3, there is likely going to be pressure to maintain 0.2.

@briansmith
Copy link
Contributor

There was a proposal to rename getrandom(_uninit) to fill(_uninit),

It would be nice but (a) it can be done in a backward-compatible way by renaming it to fill and providing a wrapper named getrandom[_uninit] that is optionally marked #[deprecated].

#365

I think nonblocking getrandom should be a separate API and so it could be added in a SemVer-capable manner. And/or we could implement the old API in terms of the new API, optionally eventually marking the old API #[deprecated].

Associated Error constants could also use a bit of work, e.g. I am not sure it's worth to expose constants like VXWORKS_RAND_SECURE on all platforms.

Better to discuss it in its own issue to reach a decision.

We also can use this release to reconsider our cfgs (e.g. #230 and #346). It's also worth to tweak __getrandom_custom's signature to remove this hack.

Yes, these are basically bugs that we've deferred fixes for until we make a SemVer-breaking version bump, so we should do this for 0.3.

This was referenced May 29, 2024
@josephlr
Copy link
Member Author

I opened #438 and #439 to discuss breaking changes to the API and feature flags respectively. I'm convinced by the arguments above, we should release these changes as a breaking change.

  • What will we cap the MSRV to for the 0.3 series?

Personally, I think that we should cap the MSRV for the upcoming release at 1.57 (1.56 is Rust 2021, and 1.57 gives us panicking in const which is super nice for static checks). My preference would actually be to release these upcoming changes as 1.0. I know @newpavlov wants to wait for MSRV-aware dependency resolution, but I'm not sure why we should block releasing 1.0 on that feature.

For 1.0 and onward, we can just say that removing platform support and increasing the MSRV can be done in a minor version, and that:

  • we won't drop platform support until Rust libstd does
  • our MSRV will always be "conservative" (maybe 2 years of old compiler versions?)

That way folks can put getrandom = "1.0" in their dependency file if they are fine with an MSRV of "2 years ago" and if they really want to for-sure not be broken, they can do getrandom = "~1.0"

  • When we add new OS support for 0.3, will we backport those to 0.2? In general will we keep maintaining 0.2? Depending on the MSRV policy for 0.3, there is likely going to be pressure to maintain 0.2.

I really wouldn't want to bother back-porting changes. I think we could have backports be "contributions welcome". I'd be fine reviewing backports, but I don't want to do the backporting (especially if nobody's actually using it).

@josephlr josephlr added this to the Next Release milestone May 29, 2024
@newpavlov
Copy link
Member

I don't think we should release v1.0, not only because of the MSRV-aware resolver, but also because we will introduce new APIs. There are also potential for new features like "global crate features", which can be quite useful for us. Remember that we always can use the semver trick to release v0.3-compatible v1.0, assuming the APIs stay stable enough.

I really wouldn't want to bother back-porting changes. I think we could have backports be "contributions welcome". I'd be fine reviewing backports, but I don't want to do the backporting (especially if nobody's actually using it).

I agree with this, IIRC we acted similarly with v0.1 backports.

@briansmith
Copy link
Contributor

The main problem with trying to make the next release be 1.0: The release will be long-delayed because there's too much perfecting and too many potential compatibility-breaking changes.

I think we need to accept the MSRV increases for 0.2 and do a 0.2 release soon, so that we don't get into a state where we can't do a release because master isn't good enough to be 1.0.

@newpavlov
Copy link
Member

@josephlr
FYI I temporarily disabled the review requirement as we discussed in the email. Unreviewed changes start from this commit and have to reviewed before we release v0.3.

@dhardy
Copy link
Member

dhardy commented Nov 25, 2024

To reply to @newpavlov here instead of in #544:

@dhardy My plan is to release getrandom v0.3 before before rand v0.9. You will not be able to silently migrate from v0.2 to v0.3 even with rust-random/rand#1537 because of the changes around optional backend handling.

So there are breaking changes, but not in the API. We already tell people using rand to use a direct dependency on getrandom when needing to customise the implementation so in a certain sense it would not be a breaking change to update getrandom, but on the other hand it could still cause issues.

I'd like to be able to release rand v1.0 soon after v0.9; I don't know if getrandom has plans for 1.0 soon?

@newpavlov
Copy link
Member

newpavlov commented Nov 25, 2024

I think we could release v1.0 relatively soon (in a year or two after the v0.3 release) assuming no issues will be found with the new APIs. One thing which I would like to do with v1.0 (here and with rand) is to migrate to the 2024 edition and relax our MSRV policy by telling users to rely on the new MSRV-dependent resolver. But since it involves a substantial MSRV bump, I don't think we should rush it.

@dhardy
Copy link
Member

dhardy commented Nov 27, 2024

From the docs,

The resolver is a global setting for a workspace, and the setting is ignored in dependencies.

So IIUC rand / getrandom doesn't need to use Edition 2024 or resolver=3 to benefit from this; it's enough for a dependant application to use Edition 2024 / resolver=3. This means we don't need to raise our MSRV to benefit.

With that in mind, I see no point in bumping the MSRV to use Edition 2024.


Do you have a plan for getrandom v0.3 / something you need help with? I'd like to get rand v0.9 out soon because of gen being a reserved keyword in Edition 2024.

@newpavlov
Copy link
Member

So IIUC rand / getrandom doesn't need to use Edition 2024 or resolver=3 to benefit from this

In my understanding, the problem is that resolver=3 will be available only on Rust 1.85 and later. So if rand v0.9.0 and v0.9.1 will have MSRV equal to 1.63 and 1.70, it will create problems for hypothetical users of 1.65 tollchain. So I think it's easier to just bump MSRV to 1.85 in a hypothetical v1.0 release.

Do you have a plan for getrandom v0.3 / something you need help with?

I plan to cut a pre-release after finishing #546. It's probably worth to do a PSA on URLO and Reddit, since the new opt-in target handling could be somewhat controversial with WASM developers.

@josephlr
Copy link
Member Author

josephlr commented Dec 7, 2024

Sorry @newpavlov for my delay in reviewing stuff for v0.3. I should have some time over the next week to review stuff. I started by reviewing #559 is there anything in particular I should look at next, or should I just start at the last time a reviewed a PR (which is baae5fc I think)?

@josephlr
Copy link
Member Author

josephlr commented Dec 7, 2024

I also really like the organizational changes in #538 it made the code much easier to read.

@newpavlov
Copy link
Member

@josephlr
You also can just take a look at the current state of the crate. You may have new ideas/suggestions after looking at it with fresh eyes.

@dhardy
Copy link
Member

dhardy commented Dec 11, 2024

I would like to get this + rand released early next week if possible. Are there any blocking issues / reviews required / other to-do items?

@newpavlov
Copy link
Member

I don't have anything in mind, so we only need a green light from @josephlr.

@josephlr
Copy link
Member Author

I'm mostly done with my review, just a few more things I want to think about (and get other's feedback on):

  • Should we use cargo features for purely "additive" backends?
    • This would turn wasm_js and esp_idf --cfg flags into "js" and "esp_idf" features
    • The remaining getrandom_backends would stay as-is.
    • My concern is that enabling JS support is (by far) the most common non-default configuration of this crate, and changing it might be confusing. The addition of targets is "additive" so seems like a better fit for cargo features.
  • Should we use a "nightly" feature (or "sanitizer" feature) for enabling cfg_sanitizer?

@josephlr
Copy link
Member Author

Alternatively, now that we can use getrandom_backend = "custom" for any target and it will override the default implementation, we can simply directly support wasm32-unknown-unknown with the JS implementation. Then, in the rare senario someone is using wasm32-unknown-unknown without JS, they can create a custom backend (which they would have to do regardless).

@newpavlov
Copy link
Member

newpavlov commented Dec 13, 2024

I believe we should be consistent and use configuration flags for all optional backends. Enabling the flag is relatively easy and automatically resolves potential issues with some library crates enabling the js crate feature unconditionally. During the pre-release announcement (URLO, Reddit) I did not get any push-back, so hopefully this change will not be too controversial.

Should we use a "nightly" feature (or "sanitizer" feature) for enabling cfg_sanitizer?

The problem with this hypothetical feature is that it will be used only for the sanitizer support. I think thst for sanitizer users it will be a bit easier to enable the configuration flag, than the nightly crate feature for a dependency buried deep in their dependency tree. It would've been a different story if we had some other Nightly-only functionality.

@hanna-kruppe
Copy link

hanna-kruppe commented Dec 20, 2024

As a user who struggled to keep unwanted wasm-bindgen-related code out of non-web wasm32-unknown-unknown builds in the past, I appreciate the move to a cfg flag for that. However, an unfortunate side effect of how it's done in 0.3.0-rc.0 is that Cargo.lock now contains the union of all dependencies that may be needed for any combination of cfg(..)s. That is, if I create an empty library and add getrandom as a dependency (without enabling any features):

  • With getrandom 0.2.15, there's five packages in my lockfile (my project, getrandom, cfg-if, libc, and wasi).
  • With getrandom 0.3.0-rc.0, the lockfile ends up with 33 packages in total. Including some transitive dependencies that are virtually impossible to enable (e.g., windows-sys is pulled in via rustix, but getrandom only uses rustix on Linux and Android).

The set of dependencies actually built by default hasn't changed, but polluting the lockfile like this has negative side effects. It creates more false positives for tools like cargo vendor, cargo deny, cargo audit, security advisory scans, etc. that parse the lockfile or look at the output of cargo metadata. It can also cause incorrect, unavoidable errors from Cargo itself. For example, adding a dependency on bitflags = "=2.3.3" gives the following error when attempting to regenerate the lockfile, even if the code would never be build in a configuration that actually pulls in both versions of bitflags:

error: failed to select a version for `bitflags`.
    ... required by package `rustix v0.38.38`
    ... which satisfies dependency `rustix = "^0.38.38"` of package `getrandom v0.3.0-rc.0`
    ... which satisfies dependency `getrandom = "=0.3.0-rc.0"` of package `try-getrandom v0.1.0 (/home/hanna/try-getrandom)`
versions that meet the requirements `^2.4.0` are: 2.6.0, 2.5.0, 2.4.2, 2.4.1, 2.4.0

all possible versions conflict with previously selected packages.

  previously selected package `bitflags v2.3.3`
    ... which satisfies dependency `bitflags = "=2.3.3"` of package `try-getrandom v0.1.0  (/home/hanna/try-getrandom)

Edit: also see the discussion at rust-lang/cargo#10801 and the many references to that issue across GitHub for illustration how much trouble overly broad dependencies in lockfiles can cause. At least that's a Cargo issue that should get fixed eventually, while the getrandom 0.3.0-rc.0 design interacts badly with Cargo "as designed" (one lockfile is supposed to work for any target platform and set of cfgs).

One way to mitigate these problem would be to add back feature flags in addition to the cfg(getrandom_backend=...)-gating. This has the downside that anyone who wants to use a non-default backend (that requires extra dependencies, fully custom backends can probably be exempt) has to both enable the getrandom crate's feature and set --cfg getrandom_backend=.... But from my own, biased POV that seems like the lesser evil for such a foundational, widely-used crate.

I'm raising this now because changing the rules from "set this cfg" to "set this cfg and also enable a Cargo feature" would on its face be a backwards incompatible change.

@josephlr
Copy link
Member Author

josephlr commented Dec 21, 2024

Thank you very much @hanna-kruppe for raising this concern. This is exactly the type of feedback we want for how to best enable alternative backends for this crate. I am especially sensitive to this concern because in over half of the issues I find complaining about this sort of problem, we are the ultimate cause of said problem.

I am wondering if this could be done using a single cargo feature ("use an alternative backend") + a cfg specifying which alternative backend to use.

I'll think more about how we might address this.

@newpavlov
Copy link
Member

newpavlov commented Dec 27, 2024

I think we have the following options to resolve the lock file bloat issue:

  • Make an exception for Web WASM and re-introduce the old js crate feature.
  • Move the Web WASM implementation into a separate crate and rely on the custom opt-in backend, i.e. users would have to add hypothetical getrandom_wasm_js crate to their dependency tree and explicitly "mount" functions defined in it.
  • Introduce an opt_in crate feature which would allow users to enable opt-in backends using configuration flags. In other words, Web WASM users would have to enable this feature and the getrandom_backend="wasm_js" configuration flag.

Personally, I think the latter option has the least amount of drawbacks.

@dhardy
Copy link
Member

dhardy commented Dec 27, 2024

If we use feature-flags to enable backends, a crate like rand_core should then not enable any of these features, thus the binary crate needs a direct dependency on getrandom to enable the feature flag(s) used (in addition to setting getrandom_backend). From this point-of-view the situation isn't much different than v0.2.

Can we somehow allow users to just depend on getrandom_wasm_js in addition to setting getrandom_backend="wasm_js"? Maybe, but effectively depending on getrandom_wasm_js is not much different than depending on getrandom with feature wasm_js.

Thus, I think we just have to choose which backends get enabled by default (at the cost of Cargo.lock bloat).

Qualitatively we may choose to use feature-flag the target (e.g. wasm) rather than the backend (wasm_js). In this case we can error at compile-time when the current target is not enabled, which allows better error messages.

@newpavlov
Copy link
Member

effectively depending on getrandom_wasm_js is not much different than depending on getrandom with feature wasm_js.

The difference is that users would have to explicitly define extern "Rust" function in their code. Unfortunately, defining the extern function in getrandom_wasm_js does not work, since compiler eliminates it as a dead code.

I think we just have to choose which backends get enabled by default (at the cost of Cargo.lock bloat)

I do not quite understand. We simply can not make the Web WASM backend enabled by default. Our choice is between tolerating the bloat or adding a subpar workaround to eliminate it.

Qualitatively we may choose to use feature-flag the target (e.g. wasm) rather than the backend (wasm_js).

How it's better than the proposed opt_in feature?

@dhardy
Copy link
Member

dhardy commented Dec 27, 2024

We simply can not make the Web WASM backend enabled by default.

I was assuming that keep the getrandom_backend env var in addition. Thus, the only purpose to not enabling extra backends by default is to reduce this Cargo.lock bloat.

How it's better than the proposed opt_in feature?

I don't understand what you mean exactly. A getrandom/wasm feature which includes the code for WASM/js but still requires setting getrandom_backend=wasm_js to use it? Yes, that's the same.

@dhardy
Copy link
Member

dhardy commented Jan 8, 2025

This is blocking rand v0.9, but there's a limit to how long I want to wait on this issue. It's 43 days until Rust 1.85.0 is released, presumably with edition 2024 which deprecates the gen keyword — which is still used by the last release of rand.

So shall I just go ahead and use getrandom v0.2?

@newpavlov
Copy link
Member

newpavlov commented Jan 9, 2025

The lock bloat issue looks like the only blocker. After we resolved it (either by leaving everything as-is or by introducing a crate feature workaround), we can release v0.3 right away after your and/or @josephlr approval. Personally, I would strongly prefer if rand v0.9 was using getrandom v0.3.

I will prepare an alternative PR to #574 shortly.

@newpavlov
Copy link
Member

@daxpedda
WDYT would be the best way to resolve the lock bloat issue?

@daxpedda
Copy link
Contributor

daxpedda commented Jan 14, 2025

Personally I would vote for your first suggestion:

Make an exception for Web WASM and re-introduce the old js crate feature.

Even though I also prefer the cfg approach, I don't see the advantage of requiring both, feature = "opt_in" and getrandom_backend = "wasm_js". Or did I miss the reasoning behind this idea?

Imo the best approach would be to only require an external dependency which would implicitly "mount" the backend, but as was already said:

Unfortunately, defining the extern function in getrandom_wasm_js does not work, since compiler eliminates it as a dead code.

@newpavlov
Copy link
Member

I wonder if we could ask wasm-bindgen developers to expose a public extern function for generating randomness gated on the same cfg. I experimented a bit with patched version of wasm-bindgen and it seems to work fine. Dead code elimination does not get triggered on the function because wasm-bindgen gets used by the top-level WASM project. Though it's a bit unclear how to handle the js_sys::Uint8Array case.

It could look like this:

// wasm-bindgen
#[cfg(getrandom_backend = "wasm_js")]
#[no_mangle]
unsafe extern "Rust" fn __getrandom_v03_wasm_bindgen(
    dest: *mut u8,
    len: usize,
) -> Result<(), ()> {
    // ...
}

// getrandom/wasm_js
pub fn fill_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
    extern "Rust" {
        fn __getrandom_v03_wasm_bindgen(dest: *mut u8, len: usize) -> Result<(), ()>;
    }
    unsafe { __getrandom_v03_wasm_bindgen(dest.as_mut_ptr().cast(), dest.len()) }
        .map_err(|()| Error::WEB_CRYPTO)
}

I don't see the advantage of requiring both, feature = "opt_in" and getrandom_backend = "wasm_js"

One advantage is that it prevents users from enabling the js feature in library crates. Another reason is that it provides a unified approach for potential future opt-in backends which may bring their own additional dependencies.

@daxpedda
Copy link
Contributor

I wonder if we could ask wasm-bindgen developers to expose a public extern function for generating randomness gated on the same cfg.

wasm-bindgen developer here: I'm not particularly keen about a solution like that. There are probably a bunch of crates that would like to have their workarounds in wasm-bindgen, even if getrandom is more foundational than most.

@dhardy
Copy link
Member

dhardy commented Jan 16, 2025

Personally I would vote for your first suggestion:

Make an exception for Web WASM and re-introduce the old js crate feature.

Which is #574. #580 is a little different but also acceptable.

@newpavlov
Copy link
Member

newpavlov commented Jan 17, 2025

I guess we need to make the final decision. We have 3 main options:

  1. Ignore the lock bloat issue.
  2. Re-introduce the js crate feature as an exception and continue to use getrandom_backend for other opt-in backends (Feature flags: wasm_js #574).
  3. Introduce the opt_in crate feature for all optional backends which would work together with the getrandom_backend flag (Add opt_in crate feature #580).

I am more or less fine with either, but, honestly, I am starting to lean towards the first option. We can consider wasm-bindgen as a foundation "platform" crate similar to libc. For example, with hypothetical "Web WASM" targets we would've kept it as a dependency similarly to the wasi crate despite the dependency tree bloat. The vendoring and auditing issues should be resolved by target filtering (e.g. using cargo-vendor-filterer).

Maybe we should create a poll on URLO?

@dhardy
Copy link
Member

dhardy commented Jan 18, 2025

There is a fourth option:

  1. Introduce a wasm_backend feature flag which enables the backend for selection by getrandom_backend but which does not enable it by default like js does.

I feel like this may be the best choice and would consider expanding this to cover other targets (e.g. windows_backend since windows-targets also causes some lockfile bloat). (We can also have an all_backends feature acting like opt_in.)

I wouldn't necessarily go with the result of a poll on https://users.rust-lang.org/ but it may bring salient discussion points.

@newpavlov
Copy link
Member

newpavlov commented Jan 18, 2025

Introduce a wasm_backend feature flag which enables the backend for selection by getrandom_backend but which does not enable it by default like js does.

So like the opt_in feature, but without influencing the other opt-in backends? What are advantages of this approach?

UPD: I guess the main advantage is that it would make easier to use dependence-free opt-in backends, but since the Web WASM backend will be the most popular opt-in backend, I am not sure it's worth to make a negative exception for it and risk proliferation of such features in future.

e.g. windows_backend since windows-targets also causes some lockfile bloat

I do not quite understand. Do you suggest to drop the Windows support and move it to an opt-in backend? We probably will get a significant backlash for doing that.

@dhardy
Copy link
Member

dhardy commented Jan 20, 2025

You are correct that we can't drop Windows support by default — at least, not the most common Windows variants.

I'm sure you (@newpavlov) have a better idea than I of which backends we may add/remove/replace in the future. Making the "opt in" flags specific to a backend or group of backends simply appears more future-proof than pushing all non-default backends into opt_in. (And if we want a catch-all feature, shouldn't it be named all_backends?)


I feel like the sensible options are:

  1. We keep the js feature flag as in v0.2, and do not support setting getrandom_backend to any other backend when the js feature is enabled.
  2. Similar to above, but do support setting getrandom_backend to another backend. In this case we should name the feature wasm_js or wasm_js_backend, not js.
  3. Name the feature wasm_js_backend and require using this feature and setting getrandom_backend=wasm_js in order to use JS on wasm32-unknown-unknown.

@dhardy
Copy link
Member

dhardy commented Jan 22, 2025

@newpavlov at this point I'd suggest you pick one of the three options I listed above since you are most familiar with this project. (The thing I least like about your opt_in is that's it's less clear what we are opting-in to; clearly it isn't about enabling usage of getrandom_backend as a whole, so isn't it just opting into the WASM-JS backend?)

@josephlr
Copy link
Member Author

I personally think that the simplest solution would be to have a "js" (or whatever) feature which allows enabling --cfg getrandom_backend="js". Then if someone enables the feature without turnig on the backend (or vice versa) we can give an error message explaining what to do.

@dhardy
Copy link
Member

dhardy commented Jan 22, 2025

That's fair, but in that case I think the name passed to getrandom_backend should match the feature flag's name (whether it's js or wasm_js).

@newpavlov
Copy link
Member

@dhardy
Could you tweak your PR accordingly?

@dhardy
Copy link
Member

dhardy commented Jan 25, 2025

This was closed, but 0.3 was not released yet.

@dhardy dhardy reopened this Jan 25, 2025
@newpavlov
Copy link
Member

I published v0.3.0.

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 a pull request may close this issue.

6 participants