Update leveldb - Githubissues

apache / incubator-teaclave

Apache Teaclave (incubating) is an open source universal secure computing platform, making computation on privacy-sensitive data safe and simple.

https://teaclave.apache.org

Apache License 2.0

761 stars 158 forks source link

Update leveldb #661

Closed henrysun007 closed 2 years ago

henrysun007 commented 2 years ago

Description

Update leveldb to v1.0.4 and move the code to third_party as a submodule for a clear view.

Fixes # (issue)

Type of change (select or add applied and delete the others)

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] API change with a documentation update
[ ] Additional test coverage
[X] Code cleanup or just sync with upstream third-party crates

How has this been tested?

Checklist

[X] Fork the repo and create your branch from master.
[ ] If you've added code that should be tested, add tests.
[ ] If you've changed APIs, update the documentation.
[X] Ensure the tests pass (see CI results).
[X] Make sure your code lints/format.

mssun commented 2 years ago

Hi @henrysun007, thanks for the PR.

The reason we choose to put in the common directory is that we have changed rusty_leveldb a lot, and we don't have any plan to upstream the changes. Therefore, we'd like to keep it in the Teaclave codebase as a part of Teaclave.

I saw you're using a submodule for the changes (which is not under the Apache organization). This may be not suitable for further maintenance. Can you PR the changes of the rusty_leveldb_sgx in the source code?

For the name of the common directory, if you have better suggestion. Please advise.

henrysun007 commented 2 years ago

Hi mingshen, Thanks for your comments. I am fine with the codebase and the name.

The reason for putting it as a submodule in this PR is to have a clear view on the modification to the original code. In one repo, it is not very clear what we have done to rusty-leveldb. Do you have any advice?

mssun commented 2 years ago

Yes, I still remember that Zhaofeng and I had a long discussion on this and we decided to put in the Teaclave repo. There are pros and cons. I think the most important reason is about the ownership. We want to maintain LevelDB for SGX as part of Teaclave. Definitely, this makes it not obvious to diff with the upstream, but we think it's worth to do that. The best why to upgrade LevelDB for SGX is to apply any patches from upstream to the LevelDB in Teaclave's repo. Also, to make it more clear, we can document that which version we are based on.

henrysun007 commented 2 years ago

Thanks for your patience, @mssun . Let us own the code.