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

Make cargo aware of the BROWSER environment variable. #6064

Closed
d3rped opened this issue Sep 20, 2018 · 10 comments · Fixed by #7576
Closed

Make cargo aware of the BROWSER environment variable. #6064

d3rped opened this issue Sep 20, 2018 · 10 comments · Fixed by #7576

Comments

@d3rped
Copy link

d3rped commented Sep 20, 2018

I've just started out learning rust so I heavily use the documentation of both the std libraries/cates and external crates. For that I use the doc option of rustup and cargo respectively.
rustup checks for the BROWSER environment variable and in my opinion so should cargo.

  • example:
    BROWSER=chromium cargo doc --open
  • expected behavior: the docs should be opened in chromium
  • actual behavior: the docs are opened with my (default browser) firefox
@ehuss
Copy link
Contributor

ehuss commented Sep 20, 2018

@d3rped which version of rust are you using and which platform are you on (linux, windows, etc.).

The previous behavior (in order of what is attempted):

  • Linux: BROWSER, xdg-open, gnome-open, kde-open
  • Windows: cmd /C
  • Mac: open

Starting in rust 1.30 (beta), a new strategy is used which is a bit more complex.

@d3rped
Copy link
Author

d3rped commented Sep 20, 2018

I'm using Linux (NixOS).
Thats weird... when I use the current nightly version from rustup (cargo 1.30.0-nightly (a5d8294 2018-09-13)) it does not work; however, when I use cargo from the mozilla rust overlay:

{ mozilla ? import (builtins.fetchTarball https://github.com/mozilla/nixpkgs-mozilla/archive/master.tar.gz) }:

with import <nixpkgs> { overlays = [ mozilla ]; };

stdenv.mkDerivation rec {
  name = "rust-env";
  buildInputs = [
    (rustChannelOf { date = "2018-07-24"; channel = "nightly"; }).rust
    pkgconfig openssl
  ];

  LD_LIBRARY_PATH = "/run/opengl-driver/lib:" + lib.makeLibraryPath buildInputs;  
  RUST_BACKTRACE = 1;
}

It respects the environment variable.

Installing the same version with rustup it works as well....
I guess something is broken in the current nightly....
Maybe that's related to rust-lang/rust#52873

@ehuss
Copy link
Contributor

ehuss commented Sep 21, 2018

The older nightly version you are referencing (2018-07-24) was before the new behavior was introduced. The latest nightly uses xdg-open on linux to figure out how to open the browser. I think you'll need to set the default browser in the system settings ("Default Applications > Web Browser" I think).

@alexcrichton @Seeker14491 What is the motivation for having the BROWSER environment variable be lowest priority? Normally, to me, environment variables should be highest, overriding all other settings.

@Seeker14491
Copy link
Contributor

This behavior is the result of #5888, which modified Cargo to use the opener crate I wrote. The idea was to pull out the browser-opening functionality of Cargo and rustup out into a crate to reduce code duplication. opener hasn't been integrated into rustup yet, as there were some issues to sort out in rustup first.

There were several factors that influenced the choice of not having the $BROWSER override:

  • In the old version, the override was only implemented on Linux, making it inconsistent with Windows and Mac.
  • xdg-open chooses to use $BROWSER as a fallback. It seemed redundant to add in our own $BROWSER check infront of xdg-open.
  • The override seemed very niche; I imagine you'd almost always want to open docs in your default browser. @d3rped can you explain your use case?
  • I took inspiration from the opn library, which doesn't use a $BROWSER override
  • The override adds complexity to the implementation and can lead to inconsistencies. For example, the implementation in Cargo didn't handle having multiple colon-separated browsers listed in $BROWSER, while the implementation in rustup does.

Anyways, I'd like to hear everyone's thoughts on this.

@TheNeikos wrote the pull request to Cargo adding the $BROWSER override, and @naufraghi to rustup. Maybe they have some thoughts on this?

@d3rped
Copy link
Author

d3rped commented Sep 21, 2018

Here are a few reasons why I'd like to use a different browser:

  • most of the time I've got 100+ Tabs open in firefox -> consumes less resources
  • I don't want to get distracted
  • that's kind of a dumb reason but I like the default scrolling behavior (with touchpads) of chromium (no smooth scrolling/no scrolling delay)

Alternatively I could just add a second profile to firefox.

@TheNeikos
Copy link
Contributor

I have initially written the PR because I strongly believe that ENV variables configuration should be respected before other programs are consulted. Similar to “$EDITOR”. I personally don’t use xdg-open (unless another’s program forces it) so by having it respect my browser I can be sure that it does what I want at the moment.

Configuring xdg-open is a pain though.

A possible alternative could be adding a switch specifying the browser? Or maybe return a URL that could be piped.

@naufraghi
Copy link

I proposed the PR in rustup for similar reasons, sometime I'd like to override locally the defaults (partly because I discovered just now that xdg-open uses the BROWSER variable).

But the same I think the ENV should be considered each time the code operates on a closed set of options, so that there is some way to escape from the loop in unsupported conditions without messing the PATH.

@alexcrichton
Copy link
Member

It seems reasonable to me to support BROWSER either explicitly in Cargo to transitively in the crate we use, PRs are of course always welcome!

@Seeker14491
Copy link
Contributor

If we want to support this, we should implement it in the opener crate. We should also implement the override for all platforms.

@pickfire
Copy link
Contributor

pickfire commented Dec 8, 2018

opener seems to be opening chromium when my $BROWSER is set to firefox as well that my firefox is set as the default browser in my xdg-open. This unexpected behavior affects the command cargo doc --open.

Somehow it works after I reinstall firefox.

bors added a commit that referenced this issue Nov 11, 2019
…excrichton

Add back support for `BROWSER` envvar in `cargo doc --open`.

Fixes #6064.
Fixes #5965.
@bors bors closed this as completed in 23a2efe Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants