We should check in Cargo.lock files

[dreemkiller]

Requested feature
We should check in Cargo.lock files to enable repeatable builds
Motivation
We've had a number of problems arise because dependencies were updated that broke our build process. Checking in the Cargo.lock files would lock us to specific versions, which should get rid of these problems.

Additional context

This isn't really a problem with the dependencies, per se. The dependencies should check for compatibility unless they are incrementing a major release number. However, we are stuck running on a fairly old nightly build. Unless we are able to update to a more recent stable build, it's unreasonable to expect our dependencies to check against our compiler version before releasing.

Thus, checking in our Cargo.lock files feels like the right move. It's possible that, if we ever get on a stable build version, we could change our minds on this.

[geky]

Just an FYI if anyone else sees this error during compilation in the crate value-bag:

error[E0658]: `match` is not allowed in a `const`

This seems to be a nightly/stable versioning issue thing that can be worked around by using Cargo.lock files from a build on another version. I haven't been able to narrow it down further yet.

This proposal would fix this specific build issue.

[geky]

Ah! Narrowed it down to the change from log v0.4.11 -> log v0.4.14

Anyways I'll get a PR with Cargo.lock files up shortly

[geky]

Hmm, I can push up the Cargo.lock files, but what would the real fix be? Fixing the log dependency to "=v0.4.11"?

[geky]

Wait a second, does checking in Cargo.lock files work with different configurations? It looks like the different platforms (SGX/Trustzone/Nitro) have different dependencies, and different Cargo.lock files.

[dreemkiller]

That does create a problem.

Even after we complete #81, sinaloa-test and veracruz-test will have different configurations (unless we want to create platform-specific tests - which we don't). I could see us hacking something together with fancy linking instead of build-time-configuration, but that's the type of thing that ends up being a lot of maintenance.

[geky]

Another idea, after poking around with this value-bag issue, could we instead specify all dependencies as absolute versions ("=v0.4.11")? It would make specifying dependencies a bit more annoying but should in theory give us a repeatable build.

[dreemkiller]

That works for our direct dependencies, but not our vicarious dependencies. If our direct dependencies are not specific in what they depend on, we could still have problems.

Doing that would be better than our current situation, though.

[geky]

Even after we complete #81, sinaloa-test and veracruz-test will have different configurations (unless we want to create platform-specific tests - which we don't).

Hmm, you could put the tests inside a macro, and then expand that macro based on which platform is configured. That would avoid duplication but also let us build for all platforms at once (and create a full set of Cargo.locks)?

[dreemkiller]

Macros are... icky. Especially large macros like that would end up being.

[dominic-mulligan-arm]

That works for our direct dependencies, but not our vicarious dependencies. If our direct dependencies are not specific in what they depend on, we could still have problems.

Doing that would be better than our current situation, though.

Yeah, all of our recent build problems have been related to transitive dependencies fairly deep inside our tree of dependencies that silently update and break compatibility with the version of rustc that we are using. This feels like a missing feature in cargo, namely the ability to pin to a specific compiler version (and crates advertise which compiler versions they work with).

Does anybody have an alternative to @geky's macro idea? I agree that macros are problematic (they slow down compilation times, IDEs struggle with them, etc.) but is there an alternative? This issue is becoming annoying enough that we need to come up with a fix...

[dreemkiller]

Another solution (not great, but ?maybe? I like it more than the macro idea): Check in a different Cargo.lock file for each platform(Cargo.lock.sgx, Cargo.lock.tz, Cargo.lock.nitro). Have the Makefile copy the platformCargo.lock.*file to Cargo.lock before starting.

[dominic-mulligan-arm]

I think that's a good pragmatic approach for the time being, @dreemkiller. Does anybody have any objections or see any downsides, other than it being a bit of a kludge?

[geky]

To go too far: Symlink Cargo.locks through a directory that is symlinked to the correct set of Cargo.locks?

---

I'm for this, it's a kludge but I don't see a solution that's not a kludge.

The main difference between multiple locks vs macros in tests is we'd not be compiling everything all the time. This would improve compile times but also push those sort of cross-platform compile errors into CI results.

[geky]

Another thought, Cargo.lock tracks dependencies not features. So if we remove the feature-configured dependencies we should get back to one set of Cargo.locks.

The main culprit seems to be the sgx libraries. Is this something we can compile cross-platform?

veracruz/sinaloa-test/Cargo.toml

Lines 50 to 53 in b9d3aaa

	[target.'cfg(target_arch = "x86_64")'.dependencies]
	sgx_types = { rev = "v1.1.2", git = "https://github.com/apache/teaclave-sgx-sdk.git", optional = true }
	sgx_ucrypto = { rev = "v1.1.2", git = "https://github.com/apache/teaclave-sgx-sdk.git", optional = true }
	sgx_urts = { rev = "v1.1.2", git = "https://github.com/apache/teaclave-sgx-sdk.git", optional = true }

[dominic-mulligan-arm]

This is now complete.