Skip to content

ci: use semicolons for path separation on Windows#10266

Closed
abrown wants to merge 1 commit intobytecodealliance:mainfrom
abrown:assembler-path-separator
Closed

ci: use semicolons for path separation on Windows#10266
abrown wants to merge 1 commit intobytecodealliance:mainfrom
abrown:assembler-path-separator

Conversation

@abrown
Copy link
Copy Markdown
Member

@abrown abrown commented Feb 21, 2025

The new assembler uses : to separate a list of paths containing generated code. We received a report that on Windows this could lead to build issues; this change uses ; for Windows instead.

prtest:full

The new assembler uses `:` to separate a list of paths containing
generated code. We received a report that on Windows this could lead to
build issues; this change uses `;` for Windows instead.

prtest:full
@abrown abrown requested a review from a team as a code owner February 21, 2025 00:13
@abrown abrown requested review from cfallin and removed request for a team February 21, 2025 00:13
@abrown
Copy link
Copy Markdown
Member Author

abrown commented Feb 21, 2025

It's still pretty unclear to me how this could have got past our Windows CI...

@abrown
Copy link
Copy Markdown
Member Author

abrown commented Feb 21, 2025

Perhaps something like the following is "fixing up" the paths?

defaults:
run:
shell: bash

@alexcrichton
Copy link
Copy Markdown
Member

This may technically still be broken in cross compilation scenarios due to the os of the build script differing from the os of the target. To sidestep all encoding issues could the array be generated in a rust file in the build script which is included with a macro?

@abrown abrown enabled auto-merge February 21, 2025 00:33
@abrown abrown added this pull request to the merge queue Feb 21, 2025
@abrown abrown removed this pull request from the merge queue due to a manual request Feb 21, 2025
abrown added a commit to abrown/wasmtime that referenced this pull request Feb 21, 2025
This fix is necessary for Windows users who may be using absolute-path
target directories: the previous solution, separating the paths by `:`,
runs into issues with Windows absolute paths (e.g., `C:\...`). This
change is similar to bytecodealliance#10266 but should avoid any further OS
compatibility issues during a hypothetical cross-compilation.

prtest:full
github-merge-queue bot pushed a commit that referenced this pull request Feb 21, 2025
* ci: generate a list of generated files

This fix is necessary for Windows users who may be using absolute-path
target directories: the previous solution, separating the paths by `:`,
runs into issues with Windows absolute paths (e.g., `C:\...`). This
change is similar to #10266 but should avoid any further OS
compatibility issues during a hypothetical cross-compilation.

prtest:full

* fix: debug string
@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Feb 21, 2025
@alexcrichton
Copy link
Copy Markdown
Member

Superseded by #10267

@abrown abrown deleted the assembler-path-separator branch February 21, 2025 16:46
abrown added a commit to abrown/wasmtime that referenced this pull request Feb 21, 2025
* ci: generate a list of generated files

This fix is necessary for Windows users who may be using absolute-path
target directories: the previous solution, separating the paths by `:`,
runs into issues with Windows absolute paths (e.g., `C:\...`). This
change is similar to bytecodealliance#10266 but should avoid any further OS
compatibility issues during a hypothetical cross-compilation.

prtest:full

* fix: debug string
abrown added a commit that referenced this pull request Feb 21, 2025
* ci: generate a list of generated files (#10267)

* ci: generate a list of generated files

This fix is necessary for Windows users who may be using absolute-path
target directories: the previous solution, separating the paths by `:`,
runs into issues with Windows absolute paths (e.g., `C:\...`). This
change is similar to #10266 but should avoid any further OS
compatibility issues during a hypothetical cross-compilation.

prtest:full

* fix: debug string

* Add clippy-required annotation

* Add vets for recently introduced crates (#10253)

CI is currently failing due to missing audits for the following Bytecode
Alliance authored crates:

* `wasmtime-wasi-io`
* `cranelift-assembler-x64`
* `cranelift-assembler-meta`

* Add release notes for 30.0.1

* Add divider line to release notes

---------

Co-authored-by: Saúl Cabrera <saulecabrera@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift Issues related to the Cranelift code generator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants