Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upLink autoconf after installing autoconf213 on OS X #211
Conversation
Use --overwrite because autoconf and autoconf213 are installed, and either one may have been linked beforehand. They overlap in the files they provide, and Homebrew will refuse to destructively overwrite any symlinks without an explicit flag. There's no built-in Salt module for brew link, so for now this is a quick fix. This provides some idempotence for repeated Salt highstate runs by checking the directory where Homebrew keeps track of linked packages (and linking autoconf if it is not linked). It's unknown whether this approach properly handles updates to one or both packages. A better approach would be to write a custom Salt state (perhaps homebrew.linked); even if this state works properly, a custom state would be more declarative and could be upstreamed to Salt proper. Closes #210. Closes servo/servo#9504.
aneeshusa
force-pushed the
aneeshusa:link-autoconf-correctly
branch
from
9918fe8
to
7dce6fa
|
I was worried about handling version upgrades properly, so I tried testing things out manually. However, autoconf213 is already pinned to a version, and autoconf has been at 2.69 for the last ~3-4 years; homebrew won't install autoconf 2.68 anymore because of the MD5 checksum. Based on some quick searching, it doesn't seem like autoconf 2.70 will be released anytime soon. I'm comfortable punting on the question of if this handles version upgrades properly for now, as there's no good way to test it, and we can handle it if autoconf 2.70 is ever released. |
|
For future reference:
Let me know if you can think of any other cases to test by hand. |
|
That's the full set of cases! Thanks again for doing all of this work :-) And I apologize for the slow builds - we're just getting |
|
These builds are only taking ~5-6 minutes, which is pretty fast to build a whole browser :) Once I'm done testing these cases, this should be ready for review. |
|
Reviewed 1 of 1 files at r1. Comments from the review on Reviewable.io |
|
OK, all done testing. |
|
@bors-servo r+ |
|
|
Link autoconf after installing autoconf213 on OS X This needs to be tested manually on one of the Mac builders before merging. Use --overwrite because autoconf and autoconf213 are installed, and either one may have been linked beforehand. They overlap in the files they provide, and Homebrew will refuse to destructively overwrite any symlinks without an explicit flag. There's no built-in Salt module for brew link, so for now this is a quick fix. This provides some idempotence for repeated Salt highstate runs by checking the directory where Homebrew keeps track of linked packages (and linking autoconf if it is not linked). It's unknown whether this approach properly handles updates to one or both packages. A better approach would be to write a custom Salt state (perhaps homebrew.linked); even if this state works properly, a custom state would be more declarative and could be upstreamed to Salt proper. Closes servo/servo#9504. <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/211) <!-- Reviewable:end -->
|
Thanks again for all your work on this - much appreciated! |
|
|
|
Cool, I'll see about applying this to all the Mac builders. But first, I'm off to watch Spectre! |
|
I just checked and it looks like all the Mac builders already have autoconf linked. |
|
@aneeshusa Sorry, I missed the earlier comment! Yes, there are a few things where we've manually "fixed" the builders (for better or worse), so we really only notice problems like this when I spin up a new builder. I hope you enjoyed Spectre! |
aneeshusa commentedFeb 2, 2016
This needs to be tested manually on one of the Mac builders before merging.
Use --overwrite because autoconf and autoconf213 are installed, and
either one may have been linked beforehand. They overlap in the files
they provide, and Homebrew will refuse to destructively overwrite
any symlinks without an explicit flag.
There's no built-in Salt module for brew link, so for now this is a
quick fix. This provides some idempotence for repeated Salt highstate
runs by checking the directory where Homebrew keeps track of linked
packages (and linking autoconf if it is not linked). It's unknown
whether this approach properly handles updates to one or both packages.
A better approach would be to write a custom Salt state (perhaps
homebrew.linked); even if this state works properly, a custom state
would be more declarative and could be upstreamed to Salt proper.
Closes servo/servo#9504.