search

apache / incubator-teaclave-sgx-sdk

Apache Teaclave (incubating) SGX SDK helps developers to write Intel SGX applications in the Rust programming language, and also known as Rust SGX SDK.
https://teaclave.apache.org
Apache License 2.0
1.17k stars 259 forks source link

The new third party forks #115

Open elichai opened 5 years ago

elichai commented 5 years ago

Hi, First of all I want to say that I really like that you're moving into regular github forks which helps to see the code changes :)

There's one problem though, which is that we use a specific revision of this sdk in order to make sure we're on the right version always and we don't get version bumps without first making sure nothing is broken, but these new forks point to the master branch.

I'm actually not sure what the best solution is, what we do with our forks is have branches which represent the SDK version they're dependent upon.

What do you think?

Thanks, Elichai.

dingelish commented 5 years ago

I can add sgx_tstd = { git = "...", rev = "v1.0.8" } on Cargo.toml to all my forked branches. Is this a solution?

elichai commented 5 years ago

Yeah I think it is. Then I can put the commit hash of that so it will stay like that even after you update so I can manually update all the workspace together.

(although tagging the SDK version in git will make it more readable but that's good enough)

dingelish commented 5 years ago

How to tag the SDK version in git? I can do it on my forks like you :-)

elichai commented 5 years ago

Most of the time I do stuff like that: https://github.com/enigmampc/ethabi/tree/sgx-v6.1.0-v1.0.7 to represent it's sgx(might be useless because you change the repo names to sgx) ethabi version 6.1.0 and your sdk v1.0.7.

I don't mind how exactly you decide to fit it best just so in the end I can point to a specific sdk version so it will match mine :)

(i'm starting to think about making a registry for sgx crates, that way all of this things can be upstreamed there and maintained with way less work, but still thinking about how will it work and if it's worth the effort)

dingelish commented 5 years ago

Just tagged 1.0.8. Let me have dinner and then tweak the new third-party libraries :-)

dingelish commented 5 years ago

all those crates are pinned on rev = "v1.0.8", git = "..." and I'd change it every time a new version is released.

elichai commented 5 years ago

Sadly I'm having this exact issue with using https://github.com/mesalock-linux/serde-sgx and https://github.com/mesalock-linux/serde-json-sgx together and locking to revisions :(

I opened an issue on cargo for this but I don't have a good idea for a solution https://github.com/rust-lang/cargo/issues/6961

dingelish commented 5 years ago

hi @elichai , I just fixed bugs related to env vars on the most recent commit. do you prefer re-tag the v1.0.8 tag to the latest commit id, or release a new tag v1.0.9? your comments would be very helpful! thanks!

dingelish commented 5 years ago

I would like to prefer re-tag the v1.0.8 on the most recent commit -- nothing changes on docker images :-)

elichai commented 5 years ago

Sure retag :) thank you!

dingelish commented 5 years ago

no prob!

brenzi commented 5 years ago

I've found a bunch of new forks which are not tagged as suggested here: log, chrono, base64, yasna, num-bigint

What's the logic behind which ones are tagged and which ones are not? And how can I know which upstream version we're on from looking at the Cargo.toml? Should we specify that (redundantly) with version = ...:

rustls = { version = "0.15.2", rev = "v1.0.8", git = "https://github.com/mesalock-linux/rustls", branch = "mesalock_sgx", features = ["dangerous_configuration"]}

?

And what's the logic behind adding a -sgx suffix and using the upstream name but with a new branch meslaock_sgx?

dingelish commented 5 years ago

rustls/webpki/webpki-roots/sct.rs are specially treated here, because Github limits only one fork under one organization :-( these four forks have been previously maintained by mesalink on their mesalink branches. so I have no other options but maintains a new branch :-( I don't like it neither.

For all other crates, I added suffix.

I think most of the tags directly come from the original github repo. I haven't added any tags on them. But I think we need to figure out a way to tag the forks.

For now I can guarantee that the master branch of those forks can pass their tests (if ported). Don't know if you can see the CI records. And I would working on each PR (generated by Pull bot) to guarantee that after the merge the master branch still passes its tests.

The most recent problem is rand. rand is testing v0.7 on its master branch. I haven't merged it because I definitely need a tagging policy for all crates relying on it. Your comments would be very much helpful!

brenzi commented 5 years ago

It might be a bit cumbersome, but what about tagging v0.13.1_sgx1.0.8. Of course, this only makes sense if the version specified in the crate root toml matches the tag. But it would make sure we know which sdk rev it is tested against. And it quite certainly would never clash with upstream tags

akhilles commented 5 years ago

I think @brenzi's suggestion makes the most sense. Referring to crates via branch is not good for build reproducibility.