Miscellaneous improvements #78
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Cargo introduced a new dependency resolver in Rust 1.51.0 (released March 25, 2021). This resolver improves handling of cases where a dependency exists multiple times in the dependency graph with different features set, and is intended to resolve some edge cases that had been identified in the old resolver that would produce surprising or unintended behavior. Those edge cases don't necessarily impact us today, but it's good to switch to the newer resolver explicitly anyway. Signed-off-by: Andrew Lilley Brinker <[email protected]>
For the C API to handle errors, we have a constructor for GitOid called new_invalid that produces an _invalid_, _unusable_ GitOid. Specifically, this bad GitOid has a length of 0, and the hash value is a zero-initialized array. When FFI functions expected to return a GitOid fail, they can return in invalid GitOid instead, and we also expose an FFI function for that GitOid. However, this is all specific to the C FFI, and only needs to be called by our own FFI functions when they fail. It should _never_ be called directly by any Rust or C users of the GitOid crate, so we can mark it pub(crate) to avoid exposing the symbol to the linker. Signed-off-by: Andrew Lilley Brinker <[email protected]>
The `ffi/gitoid.rs` file included some `std::os::unix` imports unconditionally, which wasn't caught in CI but causes builds of the crate to fail on non-Unix systems. This is a limitation of our CI today, and in the future it would be great to have WIndows and other CI platforms set up so we can catch these issues pre-merge. This commit adds conditional compilation annotations so the imports are only done on the appropriate platform. Signed-off-by: Andrew Lilley Brinker <[email protected]>
When we're running the C test code for the FFI testing, we build some binaries which we don't want to accidentally check in, so we add them to the .gitignore here. Signed-off-by: Andrew Lilley Brinker <[email protected]>
We use a C program to validate the FFI, and the previous version did an odd thing for testing `gitoid_new_from_bytes`, where it used `sscanf` through a string into a buffer. This actually did't appear to workas intended, and I've opted to replace it with a literal buffer containing some bytes instead. I also took the opportunity to add more validity checks to some other tests, and to simplify construction of the test array by introducing a new TEST macro. Signed-off-by: Andrew Lilley Brinker <[email protected]>
This commit bumps the sha1, sha2, and url dependencies to their latest versions. For sha1 and sha2 this is just a patch version bump. For url it's a minor bump. Nothing broke with these updates, and in generally Cargo would select the latest compatible anyway. This is just good for clarity to the resolver. Signed-off-by: Andrew Lilley Brinker <[email protected]>
The `omnibor` crate doesn't actually use the hex, im, sha1, or sha2 crates anymore, and doesn't need to. All of that functionality is now in the `gitoid` crate that it depends on, so these deps can be removed. Note that the `omnibor` crate still fails to compile on Windows due to a prior issue, which can be resolved (and the deps version bumped) with the publication of a new version of the `gitoid` crate. Signed-off-by: Andrew Lilley Brinker <[email protected]>
alilleybrinker
added
type: bug
Yes, this "fixes" everything by deleting the (minimal) contents of the crate. However, the existing contents of the crate... 1. Do not do anything meaningful. 2. Overlap in a confusing way with the much-more-developed `gitoid` crate. 3. Are set to be rewritten to reflect updates to the OmniBOR specification. I personally feel it's better to be real about the state of this crate, which is that it is not meaningfully useful in the new OR old form, and move on from there, rather than publish an API that isn't meaningfully useful and is also broken. Signed-off-by: Andrew Lilley Brinker <[email protected]>
The previous version of cbindgen.toml had a couple of issues: 1. It was specifically for the `gitoid` crate but it was in the root of the project repository. 2. It still included mentioned of the old GitBOM name. 3. Really, it should refer to the crate, not to the project that manages the crate, so it should be GitOID, not OmniBOR, as the correction. This commit moves and fixes the cbindgen configuration, and updates the C testing makefile to refer to the moved file. Signed-off-by: Andrew Lilley Brinker <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR incorporates a variety of improvements, mostly to the
gitoidcrate. I did try to give meaningful commit messages to each to make it easier to review. If/when this is merged, we'll also want to publish a new version of thegitoidcrate, and then probably yank the broken-on-Windows versions, and update theomniborcrate to depend on the new version.Closes #61.