Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Comments

Quote variables in win64.mak#2499

Merged
dlang-bot merged 1 commit intodlang:masterfrom
wilzbach:quote-win64
Mar 1, 2019
Merged

Quote variables in win64.mak#2499
dlang-bot merged 1 commit intodlang:masterfrom
wilzbach:quote-win64

Conversation

@wilzbach
Copy link
Contributor

@wilzbach wilzbach commented Mar 1, 2019

The escaping is required for dlang/dmd#9398.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @wilzbach!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + druntime#2499"

@wilzbach wilzbach requested a review from andralex as a code owner March 1, 2019 01:10
@wilzbach wilzbach force-pushed the quote-win64 branch 2 times, most recently from d44b097 to bc47055 Compare March 1, 2019 01:16
@thewilsonator
Copy link
Contributor

Read auto-merge when you're done, I'll be away for a bit.

@dlang-bot dlang-bot merged commit 427df6f into dlang:master Mar 1, 2019
BUILD=release
OS=windows
DMD=$(DMD_DIR)\generated\$(OS)\$(BUILD)\$(MODEL)\dmd
DMD="$(DMD_DIR)\generated\$(OS)\$(BUILD)\$(MODEL)\dmd"
Copy link
Member

Choose a reason for hiding this comment

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

I think we should never quote when assigning to a variable, see also #2438

That also avoids the horrible quoting when invoking a sub make command, e.g. https://github.com/dlang/druntime/pull/2499/files#diff-e35cbd05831954a48feae8130a5df3f4R89 (even broken in this case).

@rainers
Copy link
Member

rainers commented Mar 1, 2019

I suspect this PR broke the build on master and other PRs. AFAICT win-farm2 uses a different VC path and fails because it contains spaces.

@wilzbach
Copy link
Contributor Author

wilzbach commented Mar 1, 2019

Argh sorry about this. Here's the revert -> #2500

We really need to move away from the auto-tester or at least make all its clients behave the same 😱

@wilzbach wilzbach deleted the quote-win64 branch March 1, 2019 08:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants