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

Upgrade 0.23, re-organise external examples and tests as external #28

Merged
merged 3 commits into from
Mar 15, 2024
Merged

Conversation

pinkforest
Copy link
Contributor

@pinkforest pinkforest commented Mar 15, 2024

  • Add self-contained deterministic validation to validate integration as CryptoProvider

dependabot bot and others added 2 commits February 29, 2024 17:47
Bumps [rustls](https://github.com/rustls/rustls) from 0.22.2 to 0.23.0.
- [Release notes](https://github.com/rustls/rustls/releases)
- [Changelog](https://github.com/rustls/rustls/blob/main/CHANGELOG.md)
- [Commits](rustls/rustls@v/0.22.2...v/0.23.0)

---
updated-dependencies:
- dependency-name: rustls
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
@pinkforest pinkforest changed the title WIP: Re-organise and add tests WIP: Upgrade 0.23, re-organise and add tests Mar 15, 2024
@tarcieri
Copy link
Member

Can you leave the rustls-rustcrypto crate in the toplevel (and exclude the others from its crate files in Cargo.tom)? It's the only actual crate of relevance in this repo.

@pinkforest
Copy link
Contributor Author

It gets really difficult to keep clean and there are relevant crates that need to be kept in lock-step
e.g. see self-tester https://github.com/yaws-rs/rustls-rustcrypto/tree/main/self-validate

I also have bunch of other crates that are validation related and need to be kept at workspace.

They have different dependencies thus workspace is easiest but yet need to be kept in lock-step

@tarcieri
Copy link
Member

It's fine to use a workspace, I'm just saying the rustls-rustcrypto crate should be kept in the root directory of the repository, and the other "helper crates" can be put in subdirectories (and excluded from the files in rustls-rustcrypto crate releases)

@pinkforest
Copy link
Contributor Author

pinkforest commented Mar 15, 2024

On other hand it could be nice to solve the API lock-step maintenance issue ?

How about we have "API-crate" for 0.23, 0.22 that proxies to logic-crate given relevant API compats ?

EDIT: This would save from running contributor-confusing / complicated git branch for diff release streams / backports

@tarcieri
Copy link
Member

I'd prefer not to overcomplicate things. I think we only need one crate as a release artifact (rustls-rustcrypto) and we should be targeting rustls v0.23.

v0.22 is obsolete and we don't need to support that.

@pinkforest
Copy link
Contributor Author

rustls is having accelerated velocity / releases and backporting may become relevant

0.23. - 15 days ago
0.22 - 2 months ago
0.21 - 1 year ago
0.20 - 2 years ago
0.19 - 3 years ago

@tarcieri
Copy link
Member

rustls-rustcrypto hasn't even seen an initial release yet. I don't think we're at the point we need to worry about maintaining extended support for old rustls versions. Let's get an initial release of the crate out first.

@newpavlov
Copy link
Member

newpavlov commented Mar 15, 2024

I agree that we should keep the old structure with workspace on top of it, e.g. rand uses a similar project structure. As for extended support of old versions, in general we do not do it outside of very serious bugs/vulnerabilities (in which case we create separate backport branches). IIUC rustls itself does not have any extended support for old versions.

@pinkforest
Copy link
Contributor Author

pinkforest commented Mar 15, 2024

Okay makes sense, now I'm chasing why rand_core doesn't enable getrandom for some reason - re: #38

This will need some API to provide own Rng

@tarcieri
Copy link
Member

You need to enable the rand_core/getrandom feature for OsRng

@pinkforest
Copy link
Contributor Author

Ah thanks! Looks like I was editing wrong file and wondering why it didn't enable it 😅

Added an issue to chase the final bit for no_std requiring API here

@tarcieri
Copy link
Member

Would it be possible to split this into two PRs: one that reorganizes the tests, and another that does the v0.23 upgrade?

That would be nice to keep the repository history clear/organized.

@pinkforest pinkforest changed the title WIP: Upgrade 0.23, re-organise and add tests Upgrade 0.23, re-organise external examples and tests as external Mar 15, 2024
@tarcieri tarcieri merged commit 504f3c3 into RustCrypto:master Mar 15, 2024
5 checks passed
@stevefan1999-personal
Copy link
Contributor

External tests doesn't seems to be running in the pipeline

@pinkforest
Copy link
Contributor Author

pinkforest commented Mar 16, 2024

that is intended because they are not in lockstep with rustls - I'm working some self-contained minimal tests atm

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.

4 participants