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

rustbuild: add support for --bindir and --sysconfdir #41853

Merged
merged 6 commits into from
May 16, 2017

Conversation

Keruspe
Copy link
Contributor

@Keruspe Keruspe commented May 9, 2017

This depends on rust-lang/rust-installer#59 and we'll need to udpate the rust-installer submodule once it gets merged for it to work

Fixes #41644

@rust-highfive
Copy link
Contributor

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@Keruspe
Copy link
Contributor Author

Keruspe commented May 9, 2017

Fixes #41644

@TimNN TimNN added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 10, 2017
@TimNN
Copy link
Contributor

TimNN commented May 10, 2017

Hi @Keruspe, thanks for the PR! We'll periodically check in on it to make sure that @alexcrichton or someone else from the team reviews it soon once it is ready.

By the way, could you add the Fixes #41644 to the top post? That way the issue will automatically be closed once this PR is merged.

r? @alexcrichton

@TimNN TimNN assigned alexcrichton and unassigned nikomatsakis May 10, 2017
@alexcrichton
Copy link
Member

Thanks! Looks good to me, r=me with the fix @TimNN mentioned and I think you'll need to remove "WIP" from the title as well

@Keruspe
Copy link
Contributor Author

Keruspe commented May 10, 2017

Well the "WIP" is there because it's missing the rust-installer part for this to actually do something, but I can remove it now I you prefer

@alexcrichton
Copy link
Member

Oh sorry about that! I just merged that PR

@Keruspe Keruspe changed the title [WIP] rustbuild: add support for --bindir and --sysconfdir rustbuild: add support for --bindir and --sysconfdir May 10, 2017
@Keruspe
Copy link
Contributor Author

Keruspe commented May 10, 2017

Thanks!

I guess this depends on #41843 now

@alexcrichton
Copy link
Member

Ok cool, I'll r+ once that merges

@Keruspe
Copy link
Contributor Author

Keruspe commented May 10, 2017

I'll probably have a few trivial conflicts to handle, do you prefer a merge or a rebase in this case?

@alexcrichton
Copy link
Member

Eh either's fine, but I'd slightly prefer a rebase personally

@bors
Copy link
Collaborator

bors commented May 16, 2017

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

@cuviper
Copy link
Member

cuviper commented May 16, 2017

Note the rust-installer submodule moved under src/tools/, and now includes your changes already as I had to update again for a different fix.

Keruspe added 5 commits May 16, 2017 08:21
Signed-off-by: Marc-Antoine Perennou <Marc-Antoine@Perennou.com>
Signed-off-by: Marc-Antoine Perennou <Marc-Antoine@Perennou.com>
Signed-off-by: Marc-Antoine Perennou <Marc-Antoine@Perennou.com>
Signed-off-by: Marc-Antoine Perennou <Marc-Antoine@Perennou.com>
Signed-off-by: Marc-Antoine Perennou <Marc-Antoine@Perennou.com>
@Keruspe
Copy link
Contributor Author

Keruspe commented May 16, 2017

@cuviper yup, saw that, thanks. Currently testing the rebased version

@Keruspe
Copy link
Contributor Author

Keruspe commented May 16, 2017

@alexcrichton for my tests to pass correctly, I would need this to be merged: alexcrichton/xz2-rs#5
It's basically the same problem we had with libgit2 too. Can we get a release of lzma-sys with that in?

@Keruspe
Copy link
Contributor Author

Keruspe commented May 16, 2017

@alexcrichton Rebased on top of the rusty rust-installer. Works fine when DESTDIR is not set in the current state, and with DESTDIR set with the xz2-rs patch applied.
We can make that a second PR if you want.

@alexcrichton
Copy link
Member

Ok I've just published a new version of lzma-sys, want to update that and I'll r+?

fixes build when DESTDIR is set

Signed-off-by: Marc-Antoine Perennou <Marc-Antoine@Perennou.com>
@Keruspe
Copy link
Contributor Author

Keruspe commented May 16, 2017

Done, thanks

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented May 16, 2017

📌 Commit 08cc29e has been approved by alexcrichton

@bors
Copy link
Collaborator

bors commented May 16, 2017

⌛ Testing commit 08cc29e with merge b28cf75...

bors added a commit that referenced this pull request May 16, 2017
rustbuild: add support for --bindir and --sysconfdir

This depends on rust-lang/rust-installer#59 and we'll need to udpate the rust-installer submodule once it gets merged for it to work

Fixes #41644
@arielb1 arielb1 added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 16, 2017
@bors
Copy link
Collaborator

bors commented May 16, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing b28cf75 to master...

@bors bors merged commit 08cc29e into rust-lang:master May 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants