router icon indicating copy to clipboard operation
router copied to clipboard

DEVELOPMENT.md could be improved

Open garypen opened this issue 3 years ago • 7 comments

Describe the bug Following along with DEVELOPMENT.md and trying to build the router on an M1 based mac, I hit a few problems/gotchas:

  1. build requires gpg.
  2. asdf doesn't work when building router-bridge github dependency
  3. M1 chips require special environment variable deployment target
  4. benchmarking targets in apollo-router-core won't compile

Item (2) is the most serious problem.

To Reproduce Steps to reproduce the behavior: Try to build the router on a development machine following the DEVELOPMENT.md. It won't build and, once you have installed gpg, you will see errors like:

The following warnings were emitted during compilation:

warning: Updating router-bridge

error: failed to run custom build command for `router-bridge v0.1.0 (https://github.com/apollographql/federation.git?rev=3aa8f3a533f19e31ab984c87a0674ec78c42ebb6#3aa8f3a5)`

Caused by:
  process didn't exit successfully: `/Users/garypen/dev/router/target/debug/build/router-bridge-976dfdeb9b18819d/build-script-build` (exit status: 101)
  --- stdout
  cargo:rerun-if-changed=js-src
  cargo:warning=Updating router-bridge

  --- stderr
  No version set for command npm
  Consider adding one of the following versions in your config file at 
  nodejs 16.9.1
  thread 'main' panicked at 'assertion failed: Command::new(&npm).current_dir(&repo_dir).args(&[\"install\"]).status().unwrap().success()', /Users/garypen/.asdf/installs/rust/1.54.0/git/checkouts/federation-320f8bad94ab52a0/3aa8f3a/router-bridge/build.rs:14:5
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...
error: build failed

Expected behavior router should build seamlessly when following instructions in DEVELOPMENT.md. The instructions should be updated and workarounds provided for problems (2), (3) and (4) until permanent solutions are identified.

Output N/A

Desktop (please complete the following information):

  • OS: Darwin Garys-MBP.mhs 21.1.0 Darwin Kernel Version 21.1.0: Wed Oct 13 17:33:01 PDT 2021; root:xnu-8019.41.5~1/RELEASE_ARM64_T6000 arm64
  • Version

Additional context N/A

garypen avatar Nov 17 '21 13:11 garypen

The docs should be updated to indicate that gpg must be installed (brew install gnupg) and workarounds for the remaining problems documented.

garypen avatar Nov 17 '21 13:11 garypen

why does it need gpg? It is not apparent in the logs, I'm curious about why that would block npm

Geal avatar Nov 17 '21 14:11 Geal

asdf install requires gpg to "verify the authenticity of downloaded archives". e.g.:

% asdf install
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  4009    0  4009    0     0  19516      0 --:--:-- --:--:-- --:--:-- 19846
You must install GnuPG to verify the authenticity of the downloaded archives before continuing with the install: https://www.gnupg.org/
% 

garypen avatar Nov 17 '21 16:11 garypen

FYI I got the same error message about No version set for command npm. I ended up trashing asdf and switched to just rustup and volta. It works now!

lennyburdette avatar Feb 23 '22 19:02 lennyburdette

FYI I got the same error message about No version set for command npm. I ended up trashing asdf and switched to just rustup and volta. It works now!

asdf is tricky to get setup and working properly. I do it by maintaining a .tool-versions in my home directory which matches the contents of .tool-versions in the router repo. Why? Because of the dependency on the router bridge repo (which I'm not sure we even need any more @o0Ignition0o ?). Anyway, glad to hear you got going.

garypen avatar Feb 24 '22 11:02 garypen

I moved away from asdf as well, possibly for the same reason (and it's hard to reason with that when we want to keep rust up to date)

IIRC we added it for the perf-tests environments and to make sure we have the correct versions set up everywhere ( cc @BrynCooke who might have more info )

o0Ignition0o avatar Feb 24 '22 11:02 o0Ignition0o

We should generally add a list of the software requirements that are necessary, per @garypen's suggestion in https://github.com/apollographql/router/pull/1605#discussion_r954700049:

  • helm-docs
  • cargo about
  • DOCKER.
  • Probably more.

abernix avatar Aug 25 '22 10:08 abernix