Wasmtime GitHub notifications bot (Oct 06 2021 at 18:45): Wasmtime GitHub notifications bot (Oct 06 2021 at 18:45):bjorn3 commented on issue #3420:
While I think this change makes sense for wasmtime, I don't think it makes much sense for cranelift, as all crates except cranelift-codegen-meta, cranelift-codegen-shared can be considered public as users of cranelift either likely need to depend on them directly (cranelift-codegen as dependency of eg cranelift-frontend) or are already re-exported by another public crate (eg cranelift-bforest and cranelift-entity are re-exported by cranelift-codegen as the bforest cq entity modules) This means that breaking changes in semver compatible versions need to prevented anyway.
Wasmtime GitHub notifications bot (Oct 06 2021 at 19:06):github-actions[bot] commented on issue #3420:
Subscribe to Label Action
cc @fitzgen, @kubkon
<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:peepmatic", "cranelift:meta", "cranelift:module", "cranelift:wasm", "wasi"Thus the following users have been cc'd because of the following labels:
- fitzgen: cranelift:area:peepmatic
- kubkon: wasi
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
Wasmtime GitHub notifications bot (Oct 18 2021 at 18:23):alexcrichton commented on issue #3420:
Sure yeah, in general version requirements primarily required for crates.io-sourced dependencies. The version requirement on a dependency declaration is only required on a
pathdependency if the crate is actually published to crates.io. This tells Cargo what the dependency will be when published to crates.io. Fordev-dependencies, however, version requirements are not required because that's purely local metadata which isn't required when publishing to crates.io (and those dep edges are dropped when publishing to crates.io because they aren't necessary).This means that any crates.io-based testing of these crates will be broken, as @bjorn3 pointed out. Personally I don't think that's a use case we want to commit to supporting because it doesn't really make much sense when we sort out the testing story ourselves.
I can add a comment but it feels a bit off to do so. This is somewhat just standard Cargo stuff. I removed the version requirements where it wasn't necessary but there's no harm in leaving it in. Leaving the versions there basically just makes for version bump diffs to be a bit noisier. Otherwise I fear that adding a comment about this is basically just commenting on the delta from before-to-after, which isn't really relevant if all you ever see is the "after" state where things don't have version dependencies.
Wasmtime GitHub notifications bot (Oct 18 2021 at 18:54):bjorn3 commented on issue #3420:
Personally I don't think that's a use case we want to commit to supporting because it doesn't really make much sense when we sort out the testing story ourselves.
Crater depends on this for testing the crates.io releases. It should still test the git repo, but won't be able to test the latest released version anymore.
Wasmtime GitHub notifications bot (Oct 19 2021 at 15:26):alexcrichton commented on issue #3420:
@bjorn3 you are correct in that this will break crater testing
Wasmtime GitHub notifications bot (Oct 25 2021 at 20:24):alexcrichton commented on issue #3420:
I've updated with what we discussed today, re-r? @cfallin
Wasmtime GitHub notifications bot (Oct 25 2021 at 20:53):github-actions[bot] commented on issue #3420:
Subscribe to Label Action
cc @peterhuene
<details>
This issue or pull request has been labeled: "wasmtime:api"Thus the following users have been cc'd because of the following labels:
- peterhuene: wasmtime:api
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
Last updated: Nov 22 2024 at 17:03 UTC