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

Honor LDFLAGS when building pdl. #235

Merged
merged 1 commit into from May 10, 2018
Merged

Conversation

0-wiz-0
Copy link
Contributor

@0-wiz-0 0-wiz-0 commented May 9, 2018

This is needed e.g. when building with RELRO support.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.615% when pulling e48b200 on 0-wiz-0:master into 1d62636 on PDLPorters:master.

@mohawk2
Copy link
Member

mohawk2 commented May 10, 2018

Thanks for this! Can you confirm the ordering is right, ie that the LDFLAGS doesn't go at the end? I've restarted the one CI job that failed, since it's just showing a timeout which is vastly unlikely to be your fault!

@0-wiz-0
Copy link
Contributor Author

0-wiz-0 commented May 10, 2018

I've used it for -Wl,-z options and it worked fine where it was.

@mohawk2
Copy link
Member

mohawk2 commented May 10, 2018

Looks good to me. @devel-chm want to merge?

@zmughal
Copy link
Member

zmughal commented May 10, 2018

Since the above checks do not show the passing AppVeyor build (probably a notification hiccup), here it is: https://ci.appveyor.com/project/zmughal/pdl/build/1.0.627.

@zmughal
Copy link
Member

zmughal commented May 10, 2018

This seems reasonable.

Merge: Aye.

@mohawk2 mohawk2 merged commit f5457e4 into PDLPorters:master May 10, 2018
@mohawk2
Copy link
Member

mohawk2 commented May 10, 2018

@0-wiz-0 Thanks very much for this! Going forwards, it will be better if you do it on a local branch that's not master, which makes rebasing a bit less complicated should it be needed :-)

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.

None yet

4 participants