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

mk: fix undefined variable warnings #30367

Merged
merged 1 commit into from
Feb 1, 2016
Merged

Conversation

tamird
Copy link
Contributor

@tamird tamird commented Dec 13, 2015

Some of this is scary stuff. Probably time to lint against this.

Found with make --warn-undefined-variables.

r? @alexcrichton

@tamird tamird force-pushed the fix-makefile-bugs branch 12 times, most recently from cc8f66d to 62c36fe Compare December 13, 2015 22:08
@tamird
Copy link
Contributor Author

tamird commented Dec 13, 2015

@alexcrichton this is ready for a look

@@ -679,9 +679,9 @@ case "$CFG_RELEASE_CHANNEL" in
esac

# Adjust perf and debug options for debug mode
if [ -n "$CFG_ENABLE_DEBUG" ]; then
if [ $CFG_ENABLE_DEBUG -eq 1 ]; then
Copy link
Member

Choose a reason for hiding this comment

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

How come these all changed? Do shells warn about this kind of thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They don't warn; these had to change because of the addition of eval $V = 0 on L266

@bors
Copy link
Contributor

bors commented Dec 14, 2015

☔ The latest upstream changes (presumably #27937) made this pull request unmergeable. Please resolve the merge conflicts.

@tamird tamird force-pushed the fix-makefile-bugs branch 2 times, most recently from 5dfcf99 to 521ffa4 Compare December 16, 2015 19:16
@alexcrichton
Copy link
Member

The travis failure here seems worrisome?

@tamird
Copy link
Contributor Author

tamird commented Dec 16, 2015

Yeah, I'll take a look tomorrow; out of time for the day!

@tamird tamird force-pushed the fix-makefile-bugs branch 2 times, most recently from 2b7dba4 to 5123218 Compare December 17, 2015 03:33
@tamird tamird force-pushed the fix-makefile-bugs branch 3 times, most recently from 62fc2df to b31fee8 Compare January 1, 2016 20:19
@tamird
Copy link
Contributor Author

tamird commented Jan 2, 2016

@alexcrichton PTAL. I've taken out the configure changes which were troublesome.

@alexcrichton
Copy link
Member

@bors: r+ b31fee8

@steveklabnik
Copy link
Member

Ah wonderful! I had looked into trying to do this a while back, and didn't do it. Bravo! 👍 💯

@tamird
Copy link
Contributor Author

tamird commented Jan 12, 2016

I think the failures here are legit, looking into it

@bors
Copy link
Contributor

bors commented Jan 14, 2016

☔ The latest upstream changes (presumably #30897) made this pull request unmergeable. Please resolve the merge conflicts.

@tamird tamird force-pushed the fix-makefile-bugs branch from b31fee8 to 2b6a2db Compare January 25, 2016 16:57
@tamird
Copy link
Contributor Author

tamird commented Jan 25, 2016

Well, I wasn't able to reproduce any failures locally. Can I get another r+?

@alexcrichton
Copy link
Member

@bors: r+ 2b7a2db

@bors
Copy link
Contributor

bors commented Jan 25, 2016

🙀 2b7a2db is not a valid commit SHA. Please try again with 2b6a2db.

@alexcrichton
Copy link
Member

@bors: r+ 2b6a2db

@bors
Copy link
Contributor

bors commented Jan 25, 2016

⌛ Testing commit 2b6a2db with merge b3037dd...

@bors
Copy link
Contributor

bors commented Jan 26, 2016

💔 Test failed - auto-linux-64-x-android-t

@alexcrichton alexcrichton self-assigned this Jan 27, 2016
@bors
Copy link
Contributor

bors commented Jan 30, 2016

☔ The latest upstream changes (presumably #31274) made this pull request unmergeable. Please resolve the merge conflicts.

@tamird tamird force-pushed the fix-makefile-bugs branch from 2b6a2db to 9a00709 Compare January 31, 2016 21:40
@tamird
Copy link
Contributor Author

tamird commented Jan 31, 2016

@alexcrichton I believe I've fixed the android issue. Let's give it another go?

@alexcrichton
Copy link
Member

@bors: r+ 9a007093fa58d3a25002d2275f2fedef5af671c9

@bors
Copy link
Contributor

bors commented Feb 1, 2016

⌛ Testing commit 9a00709 with merge 4d5b8d1...

@bors
Copy link
Contributor

bors commented Feb 1, 2016

💔 Test failed - auto-linux-64-x-android-t

Some of this is scary stuff. Probably time to lint against this.

Found with `make --warn-undefined-variables`.
@tamird tamird force-pushed the fix-makefile-bugs branch from 9a00709 to d037129 Compare February 1, 2016 10:21
@tamird
Copy link
Contributor Author

tamird commented Feb 1, 2016

sigh, ifeq and $(if (...) are not interchangeable. once more, @alexcrichton? also, maybe it's ok to close this in light of #31123

@alexcrichton
Copy link
Member

Ah we can see if this works and take it from there

@bors: r+ d037129

@bors
Copy link
Contributor

bors commented Feb 1, 2016

⌛ Testing commit d037129 with merge 7cae6b5...

bors added a commit that referenced this pull request Feb 1, 2016
Some of this is scary stuff. Probably time to lint against this.

Found with `make --warn-undefined-variables`.

r? @alexcrichton
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.

5 participants