Repository navigation
fix: spell every ${mcpp.*} path token generically (#393) - #801
Open
ZheFeng7110 wants to merge 1 commit into
Open
ZheFeng7110 wants to merge 1 commit into
ZheFeng7110 wants to merge 1 commit into
Conversation
An action's `${mcpp.out_dir}`, `${mcpp.bin_dir}`, `${mcpp.compile_db}`,
`${mcpp.self}` and `${mcpp.stage_dir}` were replaced with the path's native
spelling, while `${mcpp.target_file:}` and `${mcpp.artifact:}` were already
generic. A build program that wrote `${mcpp.out_dir}/gen/x.cppm` therefore got
`C:\proj\target\...\gen/x.cppm` on Windows: one argv token carrying two
separator conventions.
The five sites use `generic_string()`, the spelling `escape_ninja_path` already
gives every ninja node name and the tool paths (the compiler, `ar`, `mcpp`
itself). POSIX output is byte-identical, so this changes no existing edge west
of Windows.
e2e 660, which runs on the Windows shard the `${mcpp.self}` token exists for,
now asserts the action rule's argv carries no backslash. docs/30 and docs/zh/30
state the rule beside the vocabulary table.
Closes mcpp-community#393.
This branch has not been deployed
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
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.
Summary
${mcpp.*}path token with forward slashes. Five sites insrc/build/prepare/plan.cppreplaced a path with its native.string()while
${mcpp.target_file:}and${mcpp.artifact:}were already.generic_string(), so the one closed vocabulary had two separatorconventions.
${mcpp.out_dir}/gen/x.cppmtherefore gotC:\proj\target\…\gen/x.cppmon Windows: one argv token with a native prefixbeside the author's
/(fix: build action 的 ${mcpp.out_dir} 替换把原生拼写拼进用户正斜杠路径,产出混合分隔符 #393).${mcpp.bin_dir},${mcpp.compile_db},${mcpp.self}and${mcpp.stage_dir}had the same shape..generic_string(), the spellingescape_ninja_pathalready gives every ninja node name and every tool path (the compiler,
ar,mcppitself), so the vocabulary agrees with the rest of the graph. POSIXoutput is byte-identical, so no existing edge changes there.
docs/30-build-mcpp.mdanddocs/zh/30-build-mcpp.mdstate the rule besidethe vocabulary table;
CHANGELOG.mdrecords it.Note on the issue's suggested fix:
make_preferredon the replacement value isa no-op here —
ctx.plan.outputDiris built withoperator/throughout, so itis already natively spelled. The mixed token comes from joining a native prefix
with the author's
/; generic is what makes the two agree.Test plan
mcpp testwith the freshly built binary:150 passed; 0 failedtests/e2e/188_build_actions.shwith the freshly builtMCPP(it drives${mcpp.out_dir},${mcpp.bin_dir}and${mcpp.target_file:}end to end)tests/e2e/660_an_action_names_the_engine_through_mcpp_self.shwith thefreshly built
MCPP; it now also asserts the action rule's argv carries nobackslash, which the windows-e2e shard this token exists for is what
exercises (on POSIX the two spellings are equal, so that leg is a no-op)
bash .github/tools/check_docs_style.shCloses #393.