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

[docs/WSL] : path to native host command in batch file #115

Open
kevinnls opened this issue Dec 26, 2021 · 7 comments
Open

[docs/WSL] : path to native host command in batch file #115

kevinnls opened this issue Dec 26, 2021 · 7 comments
Labels
controversial Something is wrong with the issue, but there is valuable discussion

Comments

@kevinnls
Copy link

General information

  • Operating system + version: Debian 11 (bullseye) on 5.10.60.1-microsoft-standard-WSL2
  • Browser + version: n/a
  • Information about the host app:
    • How did you install it? package manager - apt
    • If installed an official release, put a version ($ browserpass --version): /usr/lib/browserpass-native # -> 3.0.7
  • Information about the browser extension:
    • How did you install it? Chrome webstore
    • Browserpass extension version as reported by your browser: 3.7.2

Exact steps to reproduce the problem

  1. apt install --no-install-recommends -y webext-browserpass ### on Debian, as root

What should happen?

binary file (browserpass-native) should be installed/symlinked to /usr/bin (or /usr/local/bin)

What happened instead?

binary file (browserpass-native) exists only in /usr/lib/browserpass/browserpass-native


additional stuff

apt info webext-browserpass | grep -i version
# -> Version: 3.7.2-1+b4

dpkg-query -L webext-browserpass | grep bin 
# ->>> returns no output

command -V browserpass-linux64 browserpass-native
# -> browserpass-native not found
# -> browserpass-linux64 not found```
@erayd
Copy link
Collaborator

erayd commented Dec 27, 2021

This is intentional. It's not supposed to be in /usr/bin. This binary is an internal supporting utility, not something that a user should ever be running directly, and does not need to be (and in fact should not be) in $PATH.

Could make a reasonable argument for putting it in /usr/libexec though; that is more appropriate than the current /usr/lib.

@erayd erayd closed this as completed Dec 27, 2021
@erayd erayd added the invalid This doesn't seem right label Dec 27, 2021
@kevinnls
Copy link
Author

kevinnls commented Dec 28, 2021

hi @erayd

cc: @maximbaz

i see your point

but there are numerous instances throughout the documentation that suggest browserpass as being a command that can be run without the full path

but a very notable error would be here at the instructions for installation through WSL. the batch file that calls browserpass is looking for it in /usr/bin

bash -c "/usr/bin/browserpass-linux64 2>/dev/null"


i'd be happy to go through the docs and make a PR with the necessary changes, but... this doesn't seem to be standard through all distros. as far as i can tell, all other distro's packages install to some bin folder

links to distro package listing/specs

but

  • Gentoo has put it in /usr/libexec as you'd suggested would be more appropriate

and

  • Debian (and family) placed it in /usr/lib as i pointed out earlier

possible solutions

  1. at least wrt WSL installation, shall i add Debian-derived instructions to the FAQ section with a link near the actual installation procedure?
    • however, i believe Ubuntu is the most common distro in WSL... so perhaps the other way around?
  2. attempt writing a new make task for WSL installations that automates this (needs process elevation)

let me know your thoughts!

@erayd
Copy link
Collaborator

erayd commented Dec 29, 2021

...but there are numerous instances throughout the documentation that suggest browserpass as being a command that can be run without the full path.

Are you able to give some examples? I did a quick search through the README, but was unable to find any instances of this. If there are such, then a PR to clarify it would be helpful 🙂.

I'd be happy to go through the docs and make a PR with the necessary changes, but... this doesn't seem to be standard through all distros.

It ultimately depends on what the packager for the distro in question has decided to do. It doesn't surprise me that they are inconsistent, unfortunately. This binary shouldn't be in a bin folder anywhere, but there are plenty of people who think that any binaries should be installed to such a location - including some who are responsible for creating distro packages. It is, after all, an intuitive place to put binaries.

Debian (and family) placed it in /usr/lib as I pointed out earlier.

/usr/lib is also a historically valid location, and is where such things belonged prior to the advent of libexec. Due to some internal politics, a fair bit of stuff that would be libexec elsewhere has been packaged to lib on Debian, and thus also on Debian derivatives. The filesystem guidelines Debian uses (FHS-3.0) do also still list libexec as optional, so continuing to choose lib is still absolutely a valid place for it.


Given that the XML files which refer to the native host binary contain an absolute path, we probably shouldn't be moving it on any system with an already-existing package. That will simply break things for users. However, it could be useful to document more clearly where new packages ought to be installing it to.

WSL is a bit of an odd case - I think our documentation should probably advise the user to follow the guidelines for whatever distro they are using with WSL. However, if you have other suggestions for how best to handle this, they are most welcome!

@erayd erayd added controversial Something is wrong with the issue, but there is valuable discussion and removed invalid This doesn't seem right labels Dec 29, 2021
@erayd erayd reopened this Dec 29, 2021
@kevinnls
Copy link
Author

kevinnls commented Dec 29, 2021

just one thing i wanted to quickly comfirm, i'll respond at length later

Given that the XML files which refer to the native host binary contain an absolute path [...]

which files exactly? are you referring to these or a file in browserpass/browserpass-extension ?

(if the latter, a link to the file is appreciated)

in transit 🚈
Kevin

@kevinnls kevinnls changed the title [Debian 11/apt] : binary not installed to /usr/bin [docs/WSL] : binary path for \*-host.json Dec 29, 2021
@kevinnls kevinnls changed the title [docs/WSL] : binary path for \*-host.json [docs/WSL] : binary path for *-host.json Dec 29, 2021
@kevinnls kevinnls changed the title [docs/WSL] : binary path for *-host.json [docs/WSL] : path to native host command in batch file Dec 29, 2021
@erayd
Copy link
Collaborator

erayd commented Dec 29, 2021

are you referring to these...

Yes - specifically, to the *-host.json ones. My apologies for the confusion; I referred to them as XML files - they are actually JSON files. The actual path is substituted where %%replace%% is present in the templates.

@maximbaz
Copy link
Member

Hello!

I agree with the arguments that we shouldn't change the location of the binary.

The binary can be run with --version, but we consider this for debugging purposes, where it's clear that users should use full path if necessary; otherwise as @erayd said it should not be called directly.

The path for WSL instructions in README originally came from here, but I can see why it's wrong, sure let's fix it to be correct. Especially because you use WSL, so we trust your experiences 😉

I think our documentation should probably advise the user to follow the guidelines for whatever distro they are using with WSL

It actually already does so 😉

Follow the installation steps for the WSL distribution you are using. There is no need to configure the browser as your browser does not run in WSL.

@kevinnls
Copy link
Author

kevinnls commented Jan 1, 2022

hey folks! @erayd @maximbaz 😄
wishing everyone a happy new year 🎆


[...] numerous instances [...] documentation that suggest browserpass as being a command [...] run without the full path.

Are you able to give some examples? [...] unable to find any instances of this. [...]

i was wrong about the scale.

numerous a couple of

not opening a PR, but i added comments in source [see] if viewing in the context of the files is preferrable. full length comments are here in this comment


  1. .github/ISSUE_TEMPLATE.md#L10

If installed an official release, put a version ($ browserpass --version):

here, this seems to instruct running as a binary present in $PATH. and it does not account for the fact that the binaries are named browserpass-OS

[...] for debugging purposes, where it's clear that users should use full path if necessary; [...]

i don't believe it's clear enough in the issue template. and i was not able to find the other instance, but copying from there would be good

  1. README.md#L83

[...] if you are on macOS or FreeBSD, you probably want to install Browserpass in /usr/local/bin, therefore [...]

i don't think the bin is necessary here. but minor issue

  1. README.md#L138

bash -c "/usr/bin/browserpass-linux64 2>/dev/null"

the path for the binary would vary for each distro and hence the following exchange

[...] advise the user to follow the guidelines for whatever distro they are using with WSL

It actually already does so 😉

Follow the installation steps for the WSL distribution you are using.

but these are from the instructions for creating a bat file. not for installation of browserpass-native. and the install location of the binary seems to vary with distro.........or does it?

  1. Makefile#L5

BIN_DIR = $(DESTDIR)$(PREFIX)/bin

i tried building manually master @ d73f45c using Docker, and

  • make configure modifies the *-host files to use /*/bin... for the path
  • make install installs the binary file to /*/bin see #L124

[...] we probably shouldn't be moving it on any system with an already-existing package.
[...] I agree with the arguments that we shouldn't change the location of the binary.

to be honest, i do not see why not. would it not be a simple matter of updating the corresponding .json files? (looking to learn)

i have often seen Debian prompt me that the distribution has shipped a new version of a configuration file that i have changed locally and offer options for upgrading it; my memory is a bit fuzzy, but i believe i noticed something similar on Manjaro as well


possible solutions

  • clarify further in documentation
  • regulate installation to /*/lib from next release
  • Windows installer suggests and prompts using WSL if detected
    • creates browserpass-wsl.bat
    • changes path in *-host.json files
  • configure native path in browserpass-extension
  • new make target to install browserpass-wsl.bat file and reconfigure *-host.json process elevation requirements make it impossible [1]

looking forward to hearing your thoughts!


footnotes

[1] a work around would be to use user-level config files, but i do not know if it's that practical. after all this is configuration for the browser application itself. possible that one user would use WSL, while other would use native Windows tools.....but to what extent this is a valid scenario, i cannot say. might be simpler to configure it in the browser extensions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
controversial Something is wrong with the issue, but there is valuable discussion
Development

No branches or pull requests

3 participants