Skip to content

Comments

Update documentation of dependency setup#1900

Merged
battermann merged 5 commits intodevelopfrom
leif/getting-started
Nov 12, 2021
Merged

Update documentation of dependency setup#1900
battermann merged 5 commits intodevelopfrom
leif/getting-started

Conversation

@battermann
Copy link
Contributor

@battermann battermann commented Nov 1, 2021

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • If HTTP endpoint paths have been added or renamed, the endpoint / config-flag checklist (see Wire-employee only backend wiki page) has been followed.
  • If a cassandra schema migration has been added, I ran make git-add-cassandra-schema to update the cassandra schema documentation.
  • changelog.d contains the following bits of information:
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.
    • If new config options introduced: added usage description under docs/reference/config-options.md
    • If new config options introduced: recommended measures to be taken by on-premise instance operators.
    • If a cassandra schema migration is backwards incompatible (see also these docs), measures to be taken by on-premise instance operators are explained.
    • If a data migration (not schema migration) introduced: measures to be taken by on-premise instance operators.
    • If public end-points have been changed or added: does nginz need un upgrade?
    • If internal end-points have been added or changed: which services have to be deployed in a specific order?

@CLAassistant
Copy link

CLAassistant commented Nov 1, 2021

CLA assistant check
All committers have signed the CLA.

@jschaul jschaul requested a review from sysvinit November 3, 2021 20:10
@battermann battermann force-pushed the leif/getting-started branch from 8b81c3a to b915c44 Compare November 4, 2021 09:23
@flokli
Copy link
Contributor

flokli commented Nov 4, 2021

Can we drop the Fedora/Ubuntu instructions? Realistically everyone is using the Nix/direnv approach, and the other stuff isn't guaranteed to work (or bitrot over time).

@sysvinit
Copy link
Contributor

sysvinit commented Nov 8, 2021

The current stack configuration doesn't enable the Nix integration for me automatically on Debian (though it does for @flokli on NixOS for some reason, and I haven't looked into why). Per the stack docs, we should add nix.enable: true to stack.yaml if we want to enable the Nix integration unconditionally; without this I have to pass the --nix flag to stack to enable the Nix integration.

The direnv configuration only pulls in stack itself, and non-Haskell build dependencies like GMP and cryptobox are only pulled in through stack's Nix integration, which meant that the build failed for me due to missing dependencies without locally amending the stack config to turn that on.

I agree about the platform-specific dependency setup docs being at risk of bitrotting if nobody's regularly testing them, in which case getting rid of them is probably not a bad idea. I'm not sure whether we want to manage non-Haskell dependencies through Nix+direnv or stack+Nix, but I think that's a separate discussion.

@jschaul
Copy link
Member

jschaul commented Nov 8, 2021

Can we drop the Fedora/Ubuntu instructions? Realistically everyone is using the Nix/direnv approach, and the other stuff isn't guaranteed to work (or bitrot over time).

Hm, I think a potentially partially-out-of-date list of packages to install for some operating systems is still useful. We don't add new system-level dependencies to our services often (rather like once per year or so). At the moment the nix setup really only installs cryptobox, all the other compile-time and run-time dependencies are mentioned nowhere else and in this readme.

Taking for instance brig, it has these runtime dependencies:

❯ ldd dist/brig
	linux-vdso.so.1 (0x00007ffcff386000)
	libm.so.6 => /lib64/libm.so.6 (0x0000700a7718f000)
	libicuuc.so.67 => /lib64/libicuuc.so.67 (0x0000700a76fa4000)
	libicui18n.so.67 => /lib64/libicui18n.so.67 (0x0000700a76c9b000)
	libicudata.so.67 => /lib64/libicudata.so.67 (0x0000700a75182000)
	libsodium.so.23 => /lib64/libsodium.so.23 (0x0000700a75129000)
	libcryptobox.so => /home/user/.wire-dev/lib/libcryptobox.so (0x0000700a74e90000)
	liblzma.so.5 => /lib64/liblzma.so.5 (0x0000700a74e62000)
	libxml2.so.2 => /lib64/libxml2.so.2 (0x0000700a74cd7000)
	libstdc++.so.6 => /lib64/libstdc++.so.6 (0x0000700a74aef000)
	libz.so.1 => /lib64/libz.so.1 (0x0000700a74ad5000)
	librt.so.1 => /lib64/librt.so.1 (0x0000700a74aca000)
	libutil.so.1 => /lib64/libutil.so.1 (0x0000700a74ac5000)
	libdl.so.2 => /lib64/libdl.so.2 (0x0000700a74abc000)
	libpthread.so.0 => /lib64/libpthread.so.0 (0x0000700a74a9a000)
	libssl.so.1.1 => /lib64/libssl.so.1.1 (0x0000700a749fd000)
	libcrypto.so.1.1 => /lib64/libcrypto.so.1.1 (0x0000700a74710000)
	libgmp.so.10 => /lib64/libgmp.so.10 (0x0000700a7466b000)
	libc.so.6 => /lib64/libc.so.6 (0x0000700a744a0000)
	libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x0000700a74483000)
	/lib64/ld-linux-x86-64.so.2 (0x0000700a772ef000)

The compile-time dependencies for alpine are in https://github.com/wireapp/wire-server/blob/develop/build/alpine/Dockerfile.prebuilder and the run-time dependencies are in https://github.com/wireapp/wire-server/blob/develop/build/alpine/Dockerfile.deps - but that only covers alpine. Any new person starting to develop on wire-server will likely not run alpine, the nix dependencies don't cover even half of what's needed; so what's the harm of keeping a list of packages around for a few more OSes? If we're afraid this can bitrot, we could add them to a script file and add a docker image that installs those then tries to run make - and add the whole thing to CI to run once per week or so.

Am I the only one who thinks this is useful? Alternatively the nix setup could be changed in such a way to actually cover all dependencies; and that same setup is used for the docker images we build and ship in the end. But that's not there yet, so I'd wait before we remove some docs that are helpful to some new people starting to work on this git repository.

Copy link
Member

@jschaul jschaul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think overall this changeset is an improvement over the status quo, so feel free to merge this (even if it can be improved further)


### Nix + Direnv

Using Stack's [Nix integration](https://docs.haskellstack.org/en/stable/nix_integration/), Stack will take care of installing any system
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a note should be added that this needs to be enabled manually by passing the --nix flag to stack or set a config in stack.yaml locally. Maybe someone for whom the nix+stack setup works out of the box could add a sentence or two here.

Co-authored-by: jschaul <jschaul@users.noreply.github.com>
@battermann battermann force-pushed the leif/getting-started branch 2 times, most recently from 9c293ab to fb8374f Compare November 12, 2021 09:13
@battermann battermann marked this pull request as ready for review November 12, 2021 09:15
@battermann battermann merged commit 187d52c into develop Nov 12, 2021
@battermann battermann deleted the leif/getting-started branch November 12, 2021 09:20
Due to nginx not supporting DNS names for its list of upstream servers (unless you pay extra), the nginz-disco container is a simple bash script to do DNS lookups and write the resulting IPs to a file. Nginz reloads on changes to this file.

This is useful as a sidecar container to nginz in kubernetes. See also [wire-server-deploy/nginz](https://github.com/wireapp/wire-server-deploy/charts/nginz/)
This is useful as a sidecar container to nginz in kubernetes. See also [wire-server-deploy/nginz](https://github.com/wireapp/wire-server-deploy/charts/nginz/) <!-- todo: this link is broken >
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this chart has been merged into wire-server, so the reference should be ./charts/nginz/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants