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

Fix appending to Linux exes on Windows #148

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

grandchild
Copy link

@grandchild grandchild commented Oct 15, 2019

Right now, appending to a Linux (or in fact, any) executable with rice.exe on Windows will create an appendix containing "\" characters in the paths of the appended box files. This will fail when run on Linux, since "\" is not a path separator there.

This change, when appending on windows, will turn all "\" into "/". On the one hand this enables appending to Linux executables that will unpack properly, and on the other hand it will not phase Windows executables much, since "/" is a valid path separator on Windows as well.

@GeertJohan
Copy link
Owner

This feels really unsafe, does it really work as path separator on all versions of windows?

@grandchild
Copy link
Author

grandchild commented Oct 30, 2019

I totally get your apprehension!
I started to do this the right way and quickly realized that replacing path separators semantically would have me parse escape sequences and be probably be all sorts of buggy.

Instead:

@grandchild
Copy link
Author

The takeaway from the first link is: The Windows APIs & kernel will convert any / in paths to \.
The takeaway from the second link is: Windows (and even DOS) has always done this, the only time where it does not work is on the CMD/Batch commandline (Powershell will allow / as path separator).

@GeertJohan
Copy link
Owner

Have you tested this on a number of windows systems/versions?

@grandchild
Copy link
Author

grandchild commented Feb 2, 2020

Hey, just to let you know I've finally gotten around to testing on some Windows platforms, and will post results here shortly. So far I haven't found any problems, but I will write it up nicely, so you (or anyone) can reproduce if wanted.

I'm also writing because Windows XP support was discontinued in Go 1.11 and actually removed in 1.12. Do you want me to test Windows XP anyway? Do you support Go 1.11 or earlier? For now I am going to test Vista, 7, 8, 10 - maybe some server version just to be sure. Let me know about XP.

@GeertJohan
Copy link
Owner

Hi @grandchild, I think we can drop support for XP... Also maybe drop Vista? I mean does anyone really use that still?

@grandchild
Copy link
Author

There will be people still using both of those. Whether they are the same demographic that would use a current library in a new language like go, seems questionable, but there's always someone. I know that at my former company we still had some 32bit customers in 2019. They might be using a newer version, but it sounded a bit like they were using XP.

I had honestly forgotten a bit about this PR, but will get to it soon! Thanks for the reminder.

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.

2 participants