Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

🐛 Rome is broken on nixos #4516

Closed
1 task done
framp opened this issue May 20, 2023 · 6 comments
Closed
1 task done

🐛 Rome is broken on nixos #4516

framp opened this issue May 20, 2023 · 6 comments
Labels
S-To triage Status: user report of a possible bug that needs to be triaged

Comments

@framp
Copy link
Contributor

framp commented May 20, 2023

Environment information

CLI:
  Version:                      12.1.0
  Color support:                true

Platform:
  CPU Architecture:             x86_64
  OS:                           linux

Environment:
  ROME_LOG_DIR:                 unset
  NO_COLOR:                     unset
  TERM:                         "alacritty"
  JS_RUNTIME_VERSION:           unset
  JS_RUNTIME_NAME:              unset
  NODE_PACKAGE_MANAGER:         unset

Rome Configuration:
  Status:                       Loaded successfully
  Formatter disabled:           false
  Linter disabled:              false
  Organize imports disabled:    true
  VCS disabled:                 true

Workspace:
  Open Documents:               0

Discovering running Rome servers...

What happened?

After installing Rome on nixos via npm install (on two envs, both on my deployment / CI server and on my local development machine), I get a sigsegv as soon as I try to run node_modules/@rometools/cli-linux-x64/rome.

I can fix the issue by using the rome nix derivation and copying the executable over:

let
    pkgs = import <nixpkgs> {
    };
in
    pkgs.mkShell {
        buildInputs = [
            pkgs.nodejs_20
            pkgs.rome
        ];
        shellHook = with pkgs; ''
          cp ${pkgs.rome}/bin/rome node_modules/@rometools/cli-linux-x64/rome
        '';
    }

Expected result

  1. The executable should work or its system dependencies should be written somewhere.
  2. It would be nice to have an ENV variable (ROME_BINARY) to use instead of the shipped binary

Code of Conduct

  • I agree to follow Rome's Code of Conduct
@framp framp added the S-To triage Status: user report of a possible bug that needs to be triaged label May 20, 2023
@ematipico
Copy link
Contributor

The executable should work or its system dependencies should be written somewhere.

This highly depends on your package manager and its settings. The system dependencies are marked as optional and sometimes they don't get installed

It would be nice to have an ENV variable (ROME_BINARY) to use instead of the shipped binary

I'm not sure what you mean. Could you expand or maybe propose a suggestion via GitHub discussions?

@framp
Copy link
Contributor Author

framp commented May 23, 2023

Thanks for the quick answer and for the stellar work on Rome!

This highly depends on your package manager and its settings. The system dependencies are marked as optional and sometimes they don't get installed

I am referring to the dependencies of the rust binary: I think we're linking to a library which is commonly available on mainstream systems and my Nixos derivation doesn't have it causing the SIGSEGV. Interestingly enough the nix derivation can build Rome.

It would be nice to have an ENV variable (ROME_BINARY) to use instead of the shipped binary

I'm not sure what you mean. Could you expand or maybe propose a suggestion via GitHub discussions?

const binPath = PLATFORMS?.[platform]?.[arch];

- const binPath = PLATFORMS?.[platform]?.[arch];
+ const binPath = process.env.ROME_BINARY || PLATFORMS?.[platform]?.[arch];

framp added a commit to framp/tools that referenced this issue May 29, 2023
This is a way to temporary fix this build issue: rome#4516

I think it's a nice feature regardless, for advanced users who are experiencing issues
@ematipico
Copy link
Contributor

I am not very familiar with nix, so I don't understand the majority of the things you explained; sorry about that.

@jrouleau
Copy link

@ematipico Nix is a declarative and reproducible package manager that is increasingly used in the deployment of software, and NixOS is an operating system built upon that package manager. In Nix, the traditional unix filesystem structure of /usr/lib/..., etc. is not used in favor of everything living within its own package in /nix/store/{package}/.... If a binary is compiled with links to shared libraries that it's expecting to find in traditional locations, it won't work with Nix. The OP is requesting that the dependencies/links to shared libraries be documented to facilitate compiling with Nix and for an environment override for the node package so things like npx rome ... can be pointed to the Nix compiled binary instead of the one shipped with the package.

@framp
Copy link
Contributor Author

framp commented May 30, 2023

No worries at all! And thanks for the explanation jrouleau

One minor correction: the paths where libraries are stored in nix should be added to LD_LIBRARY_PATH so that should not cause an issue or need anything special.
Also I don't think Rome has any special dependency to blame.
Maybe a problem with glibc vs musl? (I'm running glibc)

Ultimately I wasn't able to find out what was wrong and using the nix derivation works fine, so I thought of at least flagging it here!

@framp
Copy link
Contributor Author

framp commented May 30, 2023

I looked into the issue some more and found out the root cause. Nix derivations patch executables to link the dynamic interpreter but this is not happening for binaries installed via npm

ldd node_modules/@rometools/cli-linux-x64/rome
	linux-vdso.so.1 (0x00007ffd40dac000)

A nix solution would look like this:

let
  pkgs = import <nixpkgs> {
    };
in
    pkgs.mkShell {
        buildInputs = [
            pkgs.nodejs_20
            pkgs.patchelf
        ];
        shellHook = with pkgs; ''
            patchelf --set-interpreter "$(cat $NIX_CC/nix-support/dynamic-linker)" node_modules/@rometools/cli-linux-x64/rome
        '';
    }

but it has the disadvantage of not working right after you install the package (you would need to get out and back in your shell)

If we use an env var (as per #4539), then we can use the native dependency and work all the time.

@framp framp closed this as completed Jun 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
S-To triage Status: user report of a possible bug that needs to be triaged
Projects
None yet
Development

No branches or pull requests

3 participants