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

Port rustup to Sequoia PGP. #2847

Merged
merged 1 commit into from
Dec 27, 2021
Merged

Conversation

teythoon
Copy link

@teythoon teythoon commented Sep 8, 2021

(Edited from original)

In order to port Rustup to Sequoia-PGP there are a number of steps remaining:

  • Make the CI able to build it at all
  • Verify on Windows, MacOS, and other tier one platforms
  • Verify (by adjusting the CI temporarily) that we can build all the other targets too
  • Examine licence considerations involved in using Sequoia, likely either Rustup or Sequoia would have to re-licence. (Gathering all contributors to relicence rustup might be significantly difficult, or at least very very lengthy).

If further steps are identified, the above list can be amended.

@kinnison
Copy link
Contributor

Thank you for this - naturally there are many things we need to sort out before this could be merged. I shall edit the top posting to include a checklist.

@kinnison kinnison marked this pull request as draft September 13, 2021 08:13
@teythoon
Copy link
Author

Ftr, we're porting to RustCrypto. Due to their own admission, RustCrypto is not ready for production use, but as you used rpgp before, and they also use RustCrypto, you are probably fine with Sequoia backed by RustCrypto.

@kinnison
Copy link
Contributor

That is interesting indeed, it'd reduce the complexity of the CI work for sure. Do you have an ETA on an alpha version with that support?

@teythoon
Copy link
Author

ETA on the alpha is now. There are some lxd-related errors that I don't feel responsible for.

@kinnison
Copy link
Contributor

Sadly snap isn't always reliable, I've requeued them.

The 32-bit Windows build test is worrying though:

   Compiling sha1collisiondetection v0.2.3
LLVM ERROR: Invalid LLVMRustVisibility value!
error: could not compile `sha1collisiondetection`

@teythoon
Copy link
Author

Yeah, I don't know what that is about. I can report that we do successfully compile for 32-bit Windows: https://gitlab.com/sequoia-pgp/sequoia/-/jobs/1590997603

@teythoon
Copy link
Author

(Also, sha1collisiondetiction is a crate that we use for all backends, not just for the new RustCrypto one.)

@kinnison
Copy link
Contributor

The only difference I can see is 1.55.0 vs. 1.54.0 -- can you try your build with 1.55.0 ? Maybe there's a compiler bug.

@teythoon
Copy link
Author

Seems to build the sha1cd crate fine, but fails for different reasons (new warnings being errors in our ci): https://gitlab.com/sequoia-pgp/sequoia/-/jobs/1592512848

@teythoon teythoon marked this pull request as ready for review October 5, 2021 15:05
@kinnison kinnison closed this Oct 5, 2021
@kinnison kinnison reopened this Oct 5, 2021
Copy link
Contributor

@kinnison kinnison left a comment

Choose a reason for hiding this comment

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

I think this looks good, but we do want the CI to run. If necessary I'll see if I can trigger things.

@teythoon
Copy link
Author

As of sequoia-openpgp 1.5, we're LGPL2.0+.

@kinnison
Copy link
Contributor

kinnison commented Nov 8, 2021

At minimum, rust-lang/rust#87813 is blocking this because of how sha1collisiondetection breaks the compile.

@teythoon
Copy link
Author

Now with the workaround developed by Josh Stone in https://bugzilla.redhat.com/show_bug.cgi?id=1990657.

Copy link
Contributor

@kinnison kinnison left a comment

Choose a reason for hiding this comment

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

That the full CI is green heartens me greatly. I need to spend a bit of time with the particular code bits, but I'm on the verge of approval

@kinnison kinnison merged commit 5225e87 into rust-lang:master Dec 27, 2021
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