Skip to content

Makefile: Get git revision correctly on Windows #9748

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

Merged
merged 1 commit into from
Oct 9, 2013

Conversation

klutzy
Copy link
Contributor

@klutzy klutzy commented Oct 7, 2013

Fixes a bug that rustc.exe -v didn't show git revision hash.
The bug is caused by that $(wildcard $(CFG_GIT)) requires
space-escaped inputs while $(CFG_GIT) is usually
C:\Program Files (x86)\Git\bin\git.exe.

@klutzy
Copy link
Contributor Author

klutzy commented Oct 7, 2013

@pnkfelix: rebased with $(subst). (previous discussion here.)
I added SPACE := and SPACE += to define a space. Found the clever hack from here.

@alexcrichton
Copy link
Member

Could you add a small comment above this section explaining why it looks a little funky? I'm having a tough time following what's escaping what :(

Fixes a bug that `rustc.exe -v` didn't show git revision hash.
The bug is caused by that `$(wildcard $(CFG_GIT))` requires
space-escaped inputs while `$(CFG_GIT)` is usually
`C:\Program Files (x86)\Git\bin\git.exe`.
@klutzy
Copy link
Contributor Author

klutzy commented Oct 9, 2013

@alexcrichton added some explanation on code. Hope it is clear now.
In short, we need to replace " " by r"\ ", but $(subst from,to,text) ignores any spaces between subst and from (this is space in this case). $(SPACE) is workaround for the issue.

bors added a commit that referenced this pull request Oct 9, 2013
Fixes a bug that `rustc.exe -v` didn't show git revision hash.
The bug is caused by that `$(wildcard $(CFG_GIT))` requires
space-escaped inputs while `$(CFG_GIT)` is usually
`C:\Program Files (x86)\Git�in\git.exe`.
@bors bors closed this Oct 9, 2013
@bors bors merged commit 6434d0b into rust-lang:master Oct 9, 2013
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.

3 participants