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

bug fix for recursive token expansion #927

Merged
merged 1 commit into from
Oct 17, 2017

Conversation

tvandijck
Copy link
Contributor

added test in gmake2.

@starkos
Copy link
Member

starkos commented Oct 17, 2017

Would it be possible to get a test that shows the recursive expansion? The gmake tests on this PR don't seem to show it?

@tvandijck
Copy link
Contributor Author

Actually the test was a proof of failure, the title of the PR may be a little off.. but it was a bug in the "recurse" function that is inside of the detoken function.
https://github.com/premake/premake-core/pull/927/files#diff-99f714b3251ec87d2c34c37944ca560cR172

At first iteration the 'field' could be nil, while in consecutive iterations it would become the right value. I have no idea if that is a lua bug, or whatever it is, but passing field down as an argument fixed it. The caching added in that function is just a performance improvement.

So the result of the test before the fix would be that the first item in the postbuildcommands was expanded correctly, while the second item would be treated like an absolute path, and chop of everything before the token expansion.

Copy link
Member

@starkos starkos left a comment

Choose a reason for hiding this comment

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

Ah, got it. Carry on then.

@tvandijck tvandijck merged commit 378c074 into premake:master Oct 17, 2017
@tvandijck tvandijck deleted the test-detoken-bug branch October 17, 2017 15:38
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