Feat/ssh submodules and core 2.6.0 - #6
Merged
Merged
Conversation
.gitmodules pointed at https://github.com/RayforceDB/{rayforce,rayforce-q}.git while every clone here uses an SSH origin, so the submodules could not be initialized without a URL rewrite. Both now use git@github.com: URLs; an existing clone picks that up with `git submodule sync --recursive`. CI needs nothing: actions/checkout (with no ssh-key input) rewrites git@github.com: to https with the job token before the submodule fetch, and persists that rewrite into each submodule's config, which is what lets scripts/check-vendored-pin.sh fetch the pinned tag inside a shallow clone. Both workflows say so next to their checkout step. People without a GitHub SSH key need `git config --global url."https://github.com/".insteadOf "git@github.com:"` to clone, and a `git = "https://..."` Cargo dependency additionally needs `net.git-fetch-with-cli = true`, since Cargo fetches a dependency's submodules itself and only honours the rewrite when it shells out to git. README, the installation page and the changelog carry both fallbacks; crates.io consumers are unaffected because the sources ship in the crate.
Move the vendored pins from core v2.5.8 (f0d4bb4) to v2.6.0 (b3e9aa1) and from rayforce-q 2.0.0 to 2.1.1, with CORE_VERSION / CORE_COMMIT in rayforce-sys/build.rs moved alongside. rayforce-q's q.h is unchanged, and the core only adds to its public header, so no bindgen allowlist changes. The core changed what a null is. ray_vec_from_raw now scans the raw payload once and raises HAS_NULLS when it already holds the type's sentinel, so a vector built from a buffer containing i64::MIN is null from construction where before the sentinel was ordinary data until set_null marked it. The empty symbol and the empty string are now their types' in-band nulls, recognised by ray_vec_is_null with or without the flag. Value::get follows the second rule with one deliberate deviation: an empty symbol or string element boxes as the empty atom rather than collapsing to the untyped null singleton. is_null_at still reports it and is_atom_null is true on the atom, so Option<String> extraction yields None, while plain String extraction keeps working; to_vec::<String>() would otherwise fail on any vector holding an empty string. vector_nulls encoded the old rule and failed against a v2.6.0 core; it now asserts the new one, plus that set_null(idx, false) is a no-op in the core and that overwriting with set is what un-nulls an element. The symbol_and_string_vectors test covers the empty-element path. The docs no longer describe a "null bitmap": nulls are sentinels behind a HAS_NULLS fast-path hint, and the doc comments on vec, get, is_null_at and set_null say what the core actually does.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
It is always better to add submodules by SSH. CI uses option to checkout via https automatically.
Also I've updated to the latest Rayforce and fixed issue relative to nulls due to changed behavior in Rayforce.