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

rustup 1.0.0 (new formula) #9617

Closed
wants to merge 1 commit into from
Closed

Conversation

pvande
Copy link
Contributor

@pvande pvande commented Feb 3, 2017

Rustup is a multi-version manager for Rust, in the same vein as
rbenv, nodenv, nodeenv, pyenv, and others.

This pull request has been opened in an effort to continue the
discussion from #5263.

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

Rustup is a multi-version manager for Rust, in the same vein as
rbenv, nodenv, nodeenv, pyenv, and others.

This pull request has been opened in an effort to continue the
discussion from Homebrew#5263.
@pvande
Copy link
Contributor Author

pvande commented Feb 3, 2017

Notable potential concerns:

  • The URL we're downloading from is not version-unique. Solving this would require a change to rustup's publishing process.
  • We're performing a manual installation of rustup, following Homebrew conventions, because the rustup-init tool provides no mechanism for us to "install" within a prefix.
  • rustup's self-management commands have not been disabled. It is unclear (to me) whether this would require a custom-built version of rustup, or whether a configuration option would be respected.
    • rustup is written in Rust, which makes building it locally a much more involved process.
    • rustup has no single, global configuration file, which makes configuring it at install-time more difficult.
  • I lack the experience with Rust to properly vet that rustup is entirely usable post-install by default.
  • I lack the experience with Rust to properly vet that rustup is entirely usable post-install for users who follow the suggestions in the caveats.

@nijikon nijikon added the new formula PR adds a new formula to Homebrew/homebrew-core label Feb 4, 2017
@MikeMcQuaid
Copy link
Member

The URL we're downloading from is not version-unique. Solving this would require a change to rustup's publishing process.

This is a hard-blocker. We need a URL that won't change.

rustup is written in Rust, which makes building it locally a much more involved process.

This is a hard-blocker. We should build this from source.

I lack the experience with Rust to properly vet that rustup is entirely usable post-install for users who follow the suggestions in the caveats.

I think the caveats should be omitted as they aren't Homebrew-specific information.

@pvande
Copy link
Contributor Author

pvande commented Feb 4, 2017

We need a URL that won't change.

I can make that suggestion to the rustup project maintainers.

We should build this from source.

Building this from source would likely require us to embed the 'rust' formula inside this one as a temporary build tool (note that this cannot simply depend on 'rust' as it deliberately conflicts with the Rust binaries).

We have another option, which is to use the officially preferred Rust installation tool to temporarily install Rust so that we can build this... Unfortunately this is that tool - so we'd end up downloading the binary version of this in order to temporarily install Rust in order to build the source for this ... so that users could actually install Rust.

Despite violating Homebrew best practices, I suspected the direct approach would be more palatable.

I think the caveats should be omitted as they aren't Homebrew-specific information.

The caveats are almost word-for-word identical with the caveats for 'rbenv', and are there for the same reason.

@MikeMcQuaid
Copy link
Member

Building this from source would likely require us to embed the 'rust' formula inside this one as a temporary build tool (note that this cannot simply depend on 'rust' as it deliberately conflicts with the Rust binaries).

We should figure out a way to use our own Rust.

Despite violating Homebrew best practices, I suspected the direct approach would be more palatable.

I'd like to see that there's a lot of demand for this before we violate our best practices.

The caveats are almost word-for-word identical with the caveats for 'rbenv', and are there for the same reason.

I'm changing those caveats too in #9662

@ilovezfs
Copy link
Contributor

ilovezfs commented Feb 5, 2017

https://www.rust-lang.org/en-US/other-installers.html

Reading that, I'm having trouble understanding why we'd carry rustup in Homebrew.

@pvande
Copy link
Contributor Author

pvande commented Feb 7, 2017

We should figure out a way to use our own Rust.

Can you point me to examples of other formulae that have a build-time-only dependency on a compiler?

I'd like to see that there's a lot of demand for this before we violate our best practices.

Fair.

I'm changing those caveats too in #9662

I'm disappointed by that, actually. Those caveats were useful if you installed (e.g.) rbenv from Homebrew expecting it to behave like a good Homebrew citizen and store its data in /usr/local. Automatically changing the default behavior of those tools could be problematic (backwards compatibility, expectations of users not familiar with Homebrew), and those caveats communicated not only that the tools were ill-behaved citizens, but how I could coerce them to behave properly.

Reading that, I'm having trouble understanding why we'd carry rustup in Homebrew.

I'm not sure to what exactly you're referring, but I can at least offer my own rationales for why I hope to see rustup in Homebrew.

As a Ruby developer, I may have projects spanning a number of old versions, and Homebrew offers no (good) way for me to switch between or install outdated – or pre-release – versions of Ruby. That's not to say that Homebrew's Ruby formula isn't useful: it's a great way to ensure that a recent Ruby exists for the purposes of fulfilling a dependency or running Ruby applications. For development purposes, however, I need the flexibility that tools like rbenv provide me.

I also want these tools to be centralized. I use Homebrew as my OS package manager because a) it's fairly comprehensive and b) I like the conventions it uses (e.g. everything under /usr/local, symlinks from the Cellar, etc.). Having tools like rbenv and ruby-build installed by Homebrew gives me a discoverable and trustworthy way of integrating them into my environment. As a further benefit, it saves me the hassle of figuring out how to integrate those tools into my Homebrew environment – the effort was taken on once, and there are (or, apparently, were) clear instructions on any further steps I might need to take.

The Rust formula is a great tool for expressing a build-time dependency for Rust, and a great tool to have available if you need to do the occasional build of a Rust project. It is not particularly useful for doing cross-compilation work, development against specific older (or nightly) versions of Rust, or working on multiple projects that compile against different versions. For those things, the Rust community has developed rustup, which succeeds multirust as the recommended way for developers to install Rust.

From a higher-level perspective, if the rustup formula doesn't belong in Homebrew Core, any argument that disqualifies it should apply equally to the *env and *-build formulae. If none of them belong in Core, they could all be moved into a "development tools" Tap – but discoverability (and backwards compatibility) will suffer unless that Tap is included by default. If those other formulae are acceptable, then rustup (the tool) should also meet the criteria for inclusion.


Note that I have no specific interest in seeing this formula included, only the tool itself. I'm new to both formula authorship and Rust, and I understand that there are a number of issues with this PR. I opened this PR in order to better facilitate the conversation, and to get help cleaning up its shortcomings.

@MikeMcQuaid
Copy link
Member

Can you point me to examples of other formulae that have a build-time-only dependency on a compiler?

# grep -E 'depends_on "(rust|go)" => :build' -r /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula | wc -l
     177

177 on Rust and Go alone.

Those caveats were useful if you installed (e.g.) rbenv from Homebrew expecting it to behave like a good Homebrew citizen and store its data in /usr/local.

It's 50:50 whether tools store things in /usr/local/var or the home directory and there's issues with using the caveated approach. Mostly, though, it's not Homebrew-specific information.

If those other formulae are acceptable, then rustup (the tool) should also meet the criteria for inclusion.

The main difference being: those other tools work just fine with our packaging recommendations as-is. You've pointed out quite a lot of issues in the original PR body (which we appreciate, thank you) with rustup being packaged in Homebrew or really by any package manager (e.g. Debian also won't subscribe to "just download this Rust binary to install this" mentality) which means that it's definitely in a different category of tools just now.

@pvande
Copy link
Contributor Author

pvande commented Feb 8, 2017

grep -E 'depends_on "(rust|go)" => :build'

Let me give that a try locally; I wasn't previously aware of depends_on ... => :build, and was concerned about the behavior of Homebrew if I depended on something that I also conflict with.

...and there's issues with using the caveated approach.

I recognize it as off-topic at this point (I'll drop the caveat from this formula), but I would be interested to hear more about these issues - this is the first I've ever heard of them.

... those other tools work just fine with our packaging recommendations as-is.

The three primary issues I identified were a) unstable download URL [which is moot if we're building from source, which is version tagged], b) we're jumping through some hoops to avoid using rustup-init [which we wouldn't use if building from source], and c) rustup has self-management commands [which they specifically permit to be disabled for use in package managers]. The additional issue of "we're downloading a pre-compiled binary" also goes away if we're building from source.

If I'm understanding properly, then, the overarching objection is that it's not being built from source - which is an objection only to this particular formula, not the tool itself (as I inferred from @ilovezfs), and one that it appears to be easy enough to resolve with depends_on ... => :build.

@ilovezfs
Copy link
Contributor

ilovezfs commented Feb 9, 2017

Let me be more explicit: I don't think rustup should be in Homebrew.

@pvande
Copy link
Contributor Author

pvande commented Feb 9, 2017

@ilovezfs That is the position I believed you held. Is there something specific about rustup that you object to that isn't present in {rb,go,py,nod}env, {ruby,python,node}-build, or multirust, or are those also objectionable?

@ilovezfs
Copy link
Contributor

ilovezfs commented Feb 9, 2017

If someone proposed adding a gccup script, which downloaded various nightly binaries conflicting with our gcc formula, I don't think that would fly. s/gcc/rust/ doesn't make it sound any less crazy to me.

@JCount
Copy link
Contributor

JCount commented Feb 9, 2017

It seems that for some reason the source tarball for rust was re-uploaded today, changing the checksum.

@ilovezfs
Copy link
Contributor

ilovezfs commented Feb 9, 2017

Lovely

@pvande
Copy link
Contributor Author

pvande commented Feb 9, 2017

@ilovezfs If you told me that this is how you switch between GCC versions in Homebrew, when gccup existed as the community-recommended approach, I'd be arguing in favor of gccup too. If GCC were doing large stable releases every month or two, had a significant number of developers depending on nightlies, and had no cross-compilation support in Homebrew, I'd question whether the gcc formula was actually useful.

You've mentioned three specific concerns – nightlies, binaries, and conflicts.

  • While rustup does install nightlies and beta versions upon request, it's a useful tool even when it's only managing stable versions and/or cross-compilation toolchains.
  • Binary "subinstallation" isn't desirable, but also isn't unique to rustup – a quick scan shows that both sbtenv and scalaenvdo that too.
  • Conflicts are an easy difference between this and (e.g.) rbenv, which avoids conflicting binaries by requiring an explicit opt-in in your shell's profile script. This makes rustup potentially problematic from the perspective of "having a Rust compiler available" for builds, but if builds are relying on the compiler in /usr/local/bin, they're going to have the same problems if I'm managing compiler versions with brew switch (or never upgrading my old compiler version).

Please help me understand what I'm missing.

@ilovezfs
Copy link
Contributor

ilovezfs commented Feb 9, 2017

this is how you switch between GCC versions in Homebrew

It's not.

had a significant number of developers depending on nightlies

If you're depending on nightlies, you should be able to handle rustup yourself.

binary "subinstallation" isn't desirable, but also isn't unique to rustup

If something is not desirable, we should do less of it, not more of it.

but if builds are relying on the compiler in /usr/local/bin

They are indeed in that the rust formula is not keg only, and so must be linked in order to be used by a build.

@MikeMcQuaid
Copy link
Member

Ok so my position is if we can install this from source, from a stable URL, without it installing to paths that will conflict with Homebrew (i.e. not to /usr/local/bin but instead /usr/local/var or ~/.rustup) and without it having any sort of self-update features possible at runtime: I'd be fine with accepting it as then it will feel similar to rbenv and friends. It looks like to do that it should only install a single binary (rustup) and any completions/manpages but not things like rustc so it doesn't conflict.

@ilovezfs
Copy link
Contributor

ilovezfs commented Feb 9, 2017

It looks like to do that it should only install a single binary (rustup)

Where (i.e., what directory location) do the rust compilers it subsequently downloads from the Internets get installed?

@MikeMcQuaid
Copy link
Member

@ilovezfs If we're looking at the rbenv model: again to either /usr/local/var or ~/.rust.

@ilovezfs
Copy link
Contributor

ilovezfs commented Feb 9, 2017

@MikeMcQuaid right, I just don't know how to interpret

Conflicts are an easy difference between this and (e.g.) rbenv, which avoids conflicting binaries by requiring an explicit opt-in in your shell's profile script. This makes rustup potentially problematic from the perspective of "having a Rust compiler available" for builds, but if builds are relying on the compiler in /usr/local/bin, they're going to have the same problems if I'm managing compiler versions with brew switch (or never upgrading my old compiler version).

@MikeMcQuaid
Copy link
Member

Yes, I agree that that description as-is will need to be addressed somehow perhaps by putting those tools outside bin.

@pvande
Copy link
Contributor Author

pvande commented Feb 9, 2017

... if we can install this from source, from a stable URL, without it installing [conflicting binaries] and without it having any sort of self-update features possible at runtime: I'd be fine with accepting it

This seems perfectly reasonable, and very much feasible. Installing the conflicting binaries in a separate $RUSTUP_BINDIR was an option I nearly suggested last night (apologies, I wasn't as clear-headed last night as I should have been), and makes rustup opt-in in the same way the other version managers are.

How should I handle "exposing" the $RUSTUP_BINDIR?

  • Appending an export PATH=... to ~/.profile.
  • Describing the need to add $RUSTUP_BINDIR to $PATH in a Caveat.
  • Letting users figure it out on their own.

@MikeMcQuaid
Copy link
Member

How should I handle "exposing" the $RUSTUP_BINDIR?

General approach is install everything to libexec and either symlink or write a wrapper script to bin.

@fxcoudert
Copy link
Member

Closing this PR now given lack of activity. Please reopen once the formula is updated and we can review it.

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new formula PR adds a new formula to Homebrew/homebrew-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants