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.

Sign up for GitHub

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

Jump to bottom

Fix #29: use "/" in filepaths even under Windows #30

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Fix #29: use "/" in filepaths even under Windows #30

wants to merge 14 commits into from

Conversation

andreasabel
Copy link
Collaborator

Fix #29: use / in filepaths even under Windows

System.FilePath.normalise replaces slashes by backslashes.
We implement a simplified version of normalise that does the opposite.

This should make (relative) file path printed in test output more portable across OSs.

(The two commits are meaningful individually and have been tested locally.)

@andreasabel
Copy link
Collaborator Author

andreasabel commented Nov 21, 2022 •
edited
Loading

Tested here: https://github.com/andreasabel/miniagda/actions/runs/3513071912/jobs/5885471883#step:4:3260
Fixed the backslash problem for the files in subdirectories (here adm/adm1.ma):

ok test\fail.goldplate (adm/adm1.ma): stdout

(The stdout, which you cannot see here, contains something like "Parsing file adm/adm1.ma".)

Maybe we should use / instead of pathSeparator in general to streamline the output? This would then print:

ok test/fail.goldplate (adm/adm1.ma): stdout

@andreasabel andreasabel changed the title Fix #29: use / in filepaths even under Windows Fix #29: use "/" in filepaths even under Windows Nov 21, 2022
@andreasabel
Copy link
Collaborator Author

Ping @jaspervdj-luminal.

@andreasabel
Copy link
Collaborator Author

@jaspervdj-luminal : What do you think about this change?

@andreasabel
Copy link
Collaborator Author

Since I am not getting a reaction, I assume you do not mind this change, and will go ahead. I hope this is fine.

@andreasabel andreasabel force-pushed the no-normalise-filepath branch from 1d8970c to f9cbed6 Compare February 11, 2023 09:18
@andreasabel andreasabel self-assigned this Feb 11, 2023
@andreasabel andreasabel added this to the 0.3.0 milestone Feb 11, 2023
@andreasabel andreasabel force-pushed the no-normalise-filepath branch 4 times, most recently from b60096a to cf62373 Compare February 12, 2023 08:41
@andreasabel andreasabel force-pushed the no-normalise-filepath branch from 2e07e65 to bdcfde6 Compare February 26, 2023 15:36
@jaspervdj-luminal
Copy link
Contributor

@andreasabel Apologies! I will move this repo to jaspervdj/ -- Luminal/Fugue has been acquired and since I'm infrequently using the @jaspervdj-luminal account, I missed this notification.

@jaspervdj-luminal
Copy link
Contributor

I think using / in all cases sounds good -- I've been weird post-processing steps to get rid of this, and this is much cleaner.

@andreasabel
Copy link
Collaborator Author

Ah, good to hear back from you!

I have been stalled on this PR a bit since I got weird WSL errors on the GHA Windows runner. Locally, on my Windows machine, it worked correctly. I didn't know how to debug this...
The errors were raised when the testsuite tried to execute bash commands.
There hadn't been a CI for Windows before, so I guess goldplate was not intended to run on Windows originally...

@andreasabel andreasabel force-pushed the no-normalise-filepath branch from bdcfde6 to 5bc4dc8 Compare August 3, 2023 10:37
andreasabel added 12 commits July 1, 2024 22:20
@andreasabel
Fix #29: use / in filepaths even under Windows
b55c1b6 
`System.FilePath.normalise` replaces slashes by backslashes.
We implement a simplified version of `normalise` that does the opposite.

This should make (relative) file path printed in test output more portable across OSs.
@andreasabel
Bump version to 0.3.0 and CHANGELOG
e635f54
@andreasabel
CI: switch to softprops/action-gh-release, add Windows binary
10b2022 
The old release actions are deprecated.
@andreasabel
New option --numeric-version
86ebc22
@andreasabel
CI: get goldplate version from goldplate, not from github.ref
513c01b
@andreasabel
CI binaries: already save cache after successful build
8f9339f
@andreasabel
CI binaries: condition for cache save to suppress warning
85430a6 
actions/cache/save warns if we try to save to a cache key that already exists.
@andreasabel
CI binaries: setup before cache to get correct Windows stack root
bfd62d0 
On Windows, ~/.stack is not the stack root directory.
@andreasabel
CI: use bash as default shell also on Windows
8e5f62b
@andreasabel
OS-agnostic 'lines', to fix CRLF vs LF issues (Windows)
9a5215b
@andreasabel
CI binaries: setup before cache to get correct Windows stack root
c195287
@andreasabel
Pass arguments from stack/cabal test to goldplate
2581828
@andreasabel andreasabel force-pushed the no-normalise-filepath branch from 5bc4dc8 to 348ca32 Compare July 1, 2024 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@andreasabel andreasabel

Labels
platform: Windows
Projects
None yet
Milestone
0.3.0
Development

Successfully merging this pull request may close these issues.

Use slash instead of backslash under Windows
2 participants
@andreasabel @jaspervdj-luminal