msvc-wine
msvc-wine copied to clipboard
add double-quotes to shell scripts
I think this should make it possible to install it to a path that contains spaces. (Haven't tested it, had no time for that today.)
I think this should make it possible to install it to a path that contains spaces. (Haven't tested it, had no time for that today.)
Please do test, I don't want to merge this blindly if it wouldn't actually fix the thing it sets out to fix (even if more quoting generally is good).
Ok, tested it now. There was indeed some place where the spaces weren't properly handled yet. I've added replacevars.py to do correct replacements in msvcenv.sh. This could probably also be done with perl or awk, but I know how to do it in Python. I've squashed the fixes to my earlier commit so its still only one commit so I don't pollute your history.
I've added
replacevars.pyto do correct replacements inmsvcenv.sh. This could probably also be done with perl or awk, but I know how to do it in Python.
Hmm, I'm not a huge fan of adding an extra script just for doing those replacements. But in any case, I was planning another refactoring, to not hardcode BASE and BASE_UNIX as absolute paths, but calculate them at runtime, to allow moving around the installed toolchain. With that change in place (I just pushed it as f983e0be893f588a3360e7384dde930ea23b6e2b), there shouldn't be any need to change how the replacements are done, and the preexisting sed based replacement should work just fine.
I've squashed the fixes to my earlier commit so its still only one commit so I don't pollute your history.
Thanks, that's appreciated, that's my preferred form of working too!
Ah, I see there are new (conflicting) changes. I'll rebase my branch to that.
I've added
replacevars.pyto do correct replacements inmsvcenv.sh. This could probably also be done with perl or awk, but I know how to do it in Python.Hmm, I'm not a huge fan of adding an extra script just for doing those replacements. [...] With that change in place (I just pushed it as f983e0b), there shouldn't be any need to change how the replacements are done, and the preexisting
sedbased replacement should work just fine.
I'll reiterate this point just to make it clear. I don't want to add a separate script to do the replacements, when the current in-line sed command works just fine.
I understand that you are on a mission to add doublequotes everywhere to appease your personal coding standard. I'm open to adding them where they make a practical difference. I can tolerate adding them in superfluous places for consistency. But I don't want a separate standalone script for doing replacements where the current sed oneliners work just fine. (Yes I know you can come up with some theoretical issue with surprising characters in MSVCVER and SDKVER, but that's clearly out of scope here, and I'm not adding replacevars.py for taking care of that.)
Oh, sorry, I somehow totally missed that comment and only saw the inline comments. Would you instead of the mixture of bash, Python, and Perl scripts perhaps prefer just a Python script to do the job of install.sh?
I understand that you are on a mission to add doublequotes everywhere to appease your personal coding standard.
I'm not. I just mechanically did when fixing the issue. But I'm happy to adopt any coding standard of someone else's project when I'm aware of it.
Oh, sorry, I somehow totally missed that comment and only saw the inline comments. Would you instead of the mixture of bash, Python, and Perl scripts perhaps prefer just a Python script to do the job of
install.sh?
No don't feel the desire to rewrite these scripts in another language at the moment. I don't mind the existing setup at all (I hadn't even reflected on the fact that it does indeed contain a couple separate perl scripts). Just keep this PR as it is, but remove the new replacevars.py script and restore the original calls to sed (possibly with extra quoting of the regexes if you prefer that), and this should be fine with me.
Oh, sorry, I somehow totally missed that comment and only saw the inline comments. Would you instead of the mixture of bash, Python, and Perl scripts perhaps prefer just a Python script to do the job of
install.sh?No don't feel the desire to rewrite these scripts in another language at the moment. I don't mind the existing setup at all (I hadn't even reflected on the fact that it does indeed contain a couple separate perl scripts). Just keep this PR as it is, but remove the new
replacevars.pyscript and restore the original calls tosed(possibly with extra quoting of the regexes if you prefer that), and this should be fine with me.
Ping - I'd be fine with the rest of this PR if you'd skip the rewrite/addition of a new script and just keep this to adding more quoting.