Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify .pc files in place on Windows #426

Closed
wants to merge 1 commit into from

Conversation

voegelas
Copy link

@voegelas voegelas commented Mar 9, 2025

Path::Tiny's edit method creates temporary files, which may fail on GitHub's Windows runner images. Modifying pkg-config style .pc files using slurp and append works fine. Fixes #425.

Path::Tiny's edit method creates temporary files, which may fail on
GitHub's Windows runner images.  Modifying pkg-config style .pc files
using slurp and append works fine.
@Grinnz
Copy link

Grinnz commented Mar 9, 2025

There is a reason this isn't the normal way edit works; the file will be incomplete while it is being created, and could be corrupted if the edit process fails to complete.

@voegelas
Copy link
Author

voegelas commented Mar 9, 2025

append will throw an exception if a .pc file cannot be written. Right now, Build::Autoconf with edit may fail on GitHub's Windows runners. Actually, I've seen this issue in another Perl project that I forked. I then created a small workkflow that demonstrates the problem. Personally, I do not care as I've replaced Build::Autoconf with Build::MSYS. I think that it was a mistake to replace File::Spec with Path::Tiny, though.

@Grinnz
Copy link

Grinnz commented Mar 9, 2025

There is no way for append to know whether it will successfully complete the write before it truncates the existing file, that is why it is susceptible to failure; the temporary file is an atomic operation. It may be rare, but my opinion is that it is not worthwhile to introduce to every distribution using this to solve some issue with windows CI runners.

@Grinnz
Copy link

Grinnz commented Mar 9, 2025

(that is not to say it is my call, but I just want to make the impact clear)

@voegelas voegelas closed this Mar 10, 2025
@voegelas voegelas deleted the edit-with-path-tiny branch March 10, 2025 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Work around issues in GitHub's Windows runner images
2 participants