Skip to content

Conversation

Steelskin
Copy link
Collaborator

@Steelskin Steelskin commented Apr 2, 2025

vfs-overlay.yaml created as part of the stdlib step refers to paths from the swift source directory, which may not be available in subsequent jobs, causing build issues. These changes generate a vfs-overlay.yaml file on demand for jobs that need it, to work around the issue.

`vfs-overlay.yaml` refers to paths from the `swift` source directory,
which may not be available in subsequent jobs, causing build issues.
These changes make the path in `vfs-overlay.yaml` easily changeable for
subsequent jobs, fixing the issue.
@Steelskin
Copy link
Collaborator Author

@Steelskin Steelskin requested a review from compnerd April 2, 2025 15:58
@compnerd
Copy link
Owner

compnerd commented Apr 2, 2025

I'm not sure that I understand the need for this. Why not simply create the VFS Overlay from scratch if needed?

@Steelskin Steelskin changed the title Parameterize the vfs-overlay paths Create vfs-overlay.yaml on demand Apr 2, 2025
@Steelskin
Copy link
Collaborator Author

Steelskin commented Apr 2, 2025

I'm not sure that I understand the need for this. Why not simply create the VFS Overlay from scratch if needed?

I didn't want to modify the scripts too much. I changed this to create the vfs-overlay from scratch.
New try job: https://github.com/thebrowsercompany/swift-build/actions/runs/14228030312

Copy link
Owner

@compnerd compnerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than what @jeffdav says about the path handling, LGTM

Copy link
Collaborator

@jeffdav jeffdav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double check that path thing...

@Steelskin Steelskin merged commit 7e2b9e4 into main Apr 3, 2025
1 check passed
@Steelskin Steelskin deleted the fabrice/overlay-path-parameterize branch April 3, 2025 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants