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

libsodium-fork should be a submodule rather than cloned in-tree #20

Closed
algoradam opened this issue Jun 13, 2019 · 7 comments
Closed

libsodium-fork should be a submodule rather than cloned in-tree #20

algoradam opened this issue Jun 13, 2019 · 7 comments

Comments

@algoradam
Copy link
Contributor

At the moment libsodium-fork is just checked into the go-algorand tree directly. This is a maintenance burden and hurts auditability. We should switch to including libsodium-fork as a git submodule.
To avoid a significant fraction of submodule terribleness, this should probably look like the following:

  1. Add a go-algorand/submodules/ directory and put github.com/algorand/libsodium in go-algorand/submodules/libsodium-fork as a submodule.
  2. Make go-algorand/crypto/libsodium-fork a symlink to ../submodules/libsodium-fork. This will let us switch between branches without submodules and branches with submodules without horribly confusing git.
  3. Add git submodule update --init to the makefile, to be done before libsodium build
@Vervious
Copy link
Contributor

@ian-algorand

@zeldovich
Copy link
Contributor

I think submodules are somewhat clunky.

An alternative proposal would be to fork off libsodium-fork, along with a minimal Go shim around it, into a separate repo, and import that using dep vendoring (or, eventually, using Go modules). I think that fits into our dependency management plan better.

@jecassis
Copy link
Contributor

If I may add my own experience from wrestling with this same issue in the past. I would also like to preface this by saying that the code base I was working on consisted of about 120 "subrepos" and tens of millions of LOC so it might be overkill for go-algorand's purposes and due to confidentiality I can't go into too much of the implementation details.

The first thing we looked at is git submodule and almost immediately discarded that idea. As Alex said, too clunky, slow, won't play nice with branches, but worse of all requires constant developer tracking of the upstream code base and does not support versioning (at least easily).

What we came up with eventually relied on git tag and a repo tags file (not unlike NPM, except it didn't exist at that time). Then using git hook and some Perl glue we made it seamless to the developer team. This also had the advantage of extremely fast integration builds and tests using prebuilt binaries for any subrepo which was not touched (looking at Travis logs I think you already do something similar). And another nice feature was that one didn't need to git clone all 120 subrepos to bootstrap a dev environment because the hooks and the Makefile generator knew where to look.

Circling back to your dilemma, and this is where I'll show my ignorance as an outsider, these would be my questions to you: Do you really need to import the source? Can you just have prebuilt library releases in a separate repo and just link them? Does it matter if you have a monorepo or two sister repos?

If nothing else, I hope this gives you some ideas or you have a good laugh :)

@tsachiherman
Copy link
Contributor

If I may add my own experience from wrestling with this same issue in the past. I would also like to preface this by saying that the code base I was working on consisted of about 120 "subrepos" and tens of millions of LOC so it might be overkill for go-algorand's purposes and due to confidentiality I can't go into too much of the implementation details.

The first thing we looked at is git submodule and almost immediately discarded that idea. As Alex said, too clunky, slow, won't play nice with branches, but worse of all requires constant developer tracking of the upstream code base and does not support versioning (at least easily).

What we came up with eventually relied on git tag and a repo tags file (not unlike NPM, except it didn't exist at that time). Then using git hook and some Perl glue we made it seamless to the developer team. This also had the advantage of extremely fast integration builds and tests using prebuilt binaries for any subrepo which was not touched (looking at Travis logs I think you already do something similar). And another nice feature was that one didn't need to git clone all 120 subrepos to bootstrap a dev environment because the hooks and the Makefile generator knew where to look.

Circling back to your dilemma, and this is where I'll show my ignorance as an outsider, these would be my questions to you: Do you really need to import the source? Can you just have prebuilt library releases in a separate repo and just link them? Does it matter if you have a monorepo or two sister repos?

If nothing else, I hope this gives you some ideas or you have a good laugh :)

@jecassis, I think that there are two requirement that we absolutely need :

  • Traceability - we need to be able to show what is the complete source code that was used to build our product. Using mystery, non-versioned library binaries defeat that goal. Versioned libraries are good only as long as their source code is available.
  • Reusability - we need to make sure that all the resources that builds up our product and be re-evaluated from the ground-up. The best way to make sure this is the case is to compile the entire source code. ( which also give you control on which compiler you use )

Reflecting back, I can recall several companies I worked for, which failed with the above ( mystery library that cannot be rebuilt, etc.). These scenarios rarely plays into the company best interest.

@jecassis
Copy link
Contributor

Commendable goals, to be sure. Thanks for the explanation!

@Vervious Vervious modified the milestones: Backlog, Consensus Backlog Jul 11, 2019
derbear pushed a commit to derbear/go-algorand that referenced this issue Mar 24, 2020
Fix some documentation in `goal app`.
@ian-algorand
Copy link
Contributor

ian-algorand commented Apr 5, 2021

The Dev Ops team has determined that this is no longer applicable, we won't be implementing this issue. Also, no activity for 2 years, stale issue.

@ghost
Copy link

ghost commented May 4, 2021

I have some concerns about this too. It appears that you are falling behind upstream libsodium as well. Wouldn't a patchfile or something make more sense to be able to integrate with upstream directly?

@ghost ghost mentioned this issue May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants