fix(cli): preserve digits when deriving lib_name from package name#4544
fix(cli): preserve digits when deriving lib_name from package name#4544DanielChernov7 wants to merge 1 commit into
Conversation
|
Taking this - draft PR at #4544. Root cause: PR switches both call sites to |
| // produce `groth_16_verify` here, sending anchor to look for IDL/SO | ||
| // artifacts under a name Cargo never produces. | ||
| #[test] | ||
| fn lib_name_preserves_digits_in_snake_case_package_name() { |
There was a problem hiding this comment.
these regression tests passed again master aswell? can you try different words with upper and lowercases
DanielChernov7
force-pushed
the
fix/3715-lib-name-digit-boundaries
branch
from
052cea4 to
36c8d53
Compare
|
Good catch, you were right. I checked: heck 0.4 (as pinned in cli/Cargo.toml) doesn't actually split on letter↔digit boundaries, so my original test (groth16_verify → groth16_verify) passed on master too. The real divergence between heck and Cargo's default lib-target derivation is case-related - heck Force-pushed an update with four tests that actually fail on master - Groth16Verify, groth16Verify, HTTPServer, MyProgram (verified by running them against unmodified master; all four fail with e.g. left: "groth16_verify", right: "Groth16Verify"). Kept four sanity tests for the cases that already worked |
0x4ka5h
left a comment
0x4ka5h
left a comment
There was a problem hiding this comment.
i dont think we could add commit message with --trailers of ai
| pub fn get_or_create_program_id(name: &str, target_path: impl AsRef<Path>) -> Pubkey { | ||
| // Match Cargo's default lib-target naming (`-` -> `_`). Using a full | ||
| // snake_case conversion would split digit boundaries (e.g. | ||
| // `groth16_verify` -> `groth_16_verify`) and diverge from the actual |
There was a problem hiding this comment.
can you replace the groth16_verify -> groth_16_verify example with the actual case (Groth16Verify -> groth16_verify is what heck does).
There was a problem hiding this comment.
can you address this aswell? seems like you missed this one.
`Manifest::lib_name()` used `heck::to_snake_case`, which lowercases
and splits on camel-case boundaries (e.g. `Groth16Verify` ->
`groth16_verify`, `HTTPServer` -> `http_server`). Cargo's default
lib-target name derivation only replaces `-` with `_` and preserves
case, so anchor was looking for IDL / SO / keypair artifacts under a
name Cargo never produces for any package whose name contains
uppercase letters, breaking `anchor test` / `anchor build`.
Switch to `.replace('-', "_")` to match Cargo, and add regression
tests covering the four divergent cases (PascalCase with digits,
camelCase with digits, consecutive uppercase, plain PascalCase),
plus sanity tests for the cases that already worked (dash
replacement, plain snake_case pass-through, snake_case with digits,
explicit `[lib].name` override).
Closes otter-sec#3715.
DanielChernov7
force-pushed
the
fix/3715-lib-name-digit-boundaries
branch
from
36c8d53 to
f7945d0
Compare
|
Fixed the stale example in the rust_template.rs comment to use the actual heck divergence. |
0x4ka5h
left a comment
0x4ka5h
left a comment
There was a problem hiding this comment.
the changes lgtm, but i'm not sure about commit meessages @jamie-osec
|
Hi, do we have any updates so far? |
2 participants
Summary
Manifest::lib_name()andget_or_create_program_id()usedheck::to_snake_case, which inserts underscores at digit boundaries (e.g.groth16_verify->groth_16_verify). Cargo's default lib-target name derivation only replaces-with_, so anchor was looking for IDL / SO / keypair artifactsunder a name Cargo never produces, breaking
anchor test/anchor buildfor any project whose package name contains digits adjacent to letters.This PR switches both call sites to
.replace('-', "_")to match Cargo, and adds unit tests covering the digit-boundary regression, the dash → underscore case, plain snake_case pass-through, and the explicit[lib].nameoverride.Closes #3715.
Branch Target
master— this change is not breaking. The new behavior matches Cargo's default lib-target naming rule, which is what anchor's build pipeline already produces on disk.Test plan
cargo test -p anchor-cli --lib— 25 tests pass (4 new + 21 existing)cargo clippy -p anchor-cli --all-targets -- -D warnings— cleancargo +nightly fmt -p anchor-cli— applied