Skip to content

Make gcc embed rpath automatically#646

Merged
certik merged 3 commits intomasterfrom
gcc2
Feb 19, 2015
Merged

Make gcc embed rpath automatically#646
certik merged 3 commits intomasterfrom
gcc2

Conversation

@certik
Copy link
Copy Markdown
Member

@certik certik commented Feb 19, 2015

The patch adds @@artifact@@ at the proper place in the source code and we then
replace it for ${ARTIFACT} using sed.

The patch adds @@artifact@@ at the proper place in the source code and we then
replace it for ${ARTIFACT} using sed.
Comment thread pkgs/gcc/gcc.yaml Outdated
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think these two blobs can just be merged into one. I'll do it after my gcc build finishes.

Comment thread pkgs/gcc/gcc.yaml
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This line assumes that ${ARTIFACT} does not contain the | character. If it does, then the sed command will break.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@ahmadia I've searched the web and there doesn't seem to be any solution really to this, except to manually escape the variable to be substituted. I think it will also fail if the variable contains []. I am inclined to just leave this be (it should work for most paths), unless we can figure out a better solution.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can use other separator but I would assume that "|" is fairly safe for a path. It is safer than ":" which we could use if there was more than one path.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Right. The ${ARTIFACT} path can contain both : and |, so to be completely safe, one would need to escape it, but it just makes the code really messy.

@certik
Copy link
Copy Markdown
Member Author

certik commented Feb 19, 2015

I am now running my big stack using gcc built using this PR. So far it works great! I will post once it finishes.

@certik
Copy link
Copy Markdown
Member Author

certik commented Feb 19, 2015

If all works on my computer, then I would propose to merge this. Somebody with Darwin should then test the patch at #645 (comment) and submit a new PR.

@certik certik changed the title Add rpath patch Make gcc embed rpath automatically Feb 19, 2015
@certik
Copy link
Copy Markdown
Member Author

certik commented Feb 19, 2015

Ok, my (huge) stack installed, including the gcc itself (using gcc from this PR), using the gcc from this PR.

This is ready to go in. @ahmadia any objections against merging this?

@ahmadia
Copy link
Copy Markdown
Contributor

ahmadia commented Feb 19, 2015

Looks good.

@certik
Copy link
Copy Markdown
Member Author

certik commented Feb 19, 2015

Thanks, merging.

certik added a commit that referenced this pull request Feb 19, 2015
Make gcc embed rpath automatically
@certik certik merged commit 55feda2 into master Feb 19, 2015
@certik certik deleted the gcc2 branch February 19, 2015 07:01
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