From 63f545b7d7d40e12af74cecab473f5b53e5b10d6 Mon Sep 17 00:00:00 2001 From: Luiz Irber Date: Thu, 4 Feb 2021 08:21:52 -0800 Subject: [PATCH] Dev updates (configs and doc) (#1298) * new nix instructions * add dev envs installation checks to CI * update dev docs --- .github/workflows/dev_envs.yml | 64 ++++++++++++ doc/developer.md | 88 +++++++++++++---- nix.shell | 51 ---------- nix/sources.json | 38 +++++++ nix/sources.nix | 174 +++++++++++++++++++++++++++++++++ shell.nix | 22 +++++ 6 files changed, 366 insertions(+), 71 deletions(-) create mode 100644 .github/workflows/dev_envs.yml delete mode 100644 nix.shell create mode 100644 nix/sources.json create mode 100644 nix/sources.nix create mode 100644 shell.nix diff --git a/.github/workflows/dev_envs.yml b/.github/workflows/dev_envs.yml new file mode 100644 index 0000000000..26a1a17fd3 --- /dev/null +++ b/.github/workflows/dev_envs.yml @@ -0,0 +1,64 @@ +name: "Dev env instructions" +on: + pull_request: + branches: [latest] + push: + branches: [latest] +jobs: + nix: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2.3.4 + with: + fetch-depth: 0 + + - name: Cache nix store + id: cache-nix + uses: actions/cache@v2 + with: + path: /nix/store + key: nix-${{ hashFiles('shell.nix') }}-${{ hashFiles('nix/**') }} + + - uses: cachix/install-nix-action@v12 + with: + nix_path: nixpkgs=channel:nixos-20.09 + + - run: nix-shell --command "tox -e py39" + + mamba: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2.3.4 + with: + fetch-depth: 0 + + - name: cache conda + uses: actions/cache@v1 + env: + CACHE_NUMBER: 0 + with: + path: ~/conda_pkgs_dir + key: + ${{ runner.os }}-conda-${{ env.CACHE_NUMBER }}-${{ hashFiles('environment.yml') }} + + - name: setup conda + uses: conda-incubator/setup-miniconda@e23d871804685e8c52189e5bd45e9145019f10af + with: + auto-update-conda: true + python-version: 3.9 + channels: conda-forge,bioconda + miniforge-variant: Mambaforge + miniforge-version: latest + use-mamba: true + mamba-version: "*" + activate-environment: sourmash_dev + auto-activate-base: false + use-only-tar-bz2: true + + - name: install dependencies + shell: bash -l {0} + run: mamba install tox-conda rust git compilers pandoc + + - name: run tests for 3.9 + shell: bash -l {0} + run: tox -e py39 diff --git a/doc/developer.md b/doc/developer.md index ef390f62cd..c476489395 100644 --- a/doc/developer.md +++ b/doc/developer.md @@ -6,43 +6,79 @@ You can get the latest development branch with: ``` git clone https://github.com/dib-lab/sourmash.git ``` -sourmash runs under Python 3.7 and later. The base -requirements are screed and cffi, together with a Rust environment (for the -extension code). We suggest using `rustup` to install the Rust environment: +sourmash runs under Python 3.7 and later. - curl https://sh.rustup.rs -sSf | sh +We recommend using `conda` or `Nix` for setting up an environment for developing +new features, running tests and code quality checks. +Here are some suggestions on how to set them up (note: you only need one =]) -We use [`tox`](https://tox.readthedocs.io) for managing dependencies and -running tests and checks during development. -To install it, do: +### Using mamba (conda alternative) + +Follow the [installation instructions](https://github.com/conda-forge/miniforge#install) for +installing `mambaforge` (a conda distribution that uses +[`mamba`](https://github.com/TheSnakePit/mamba) +and the [`conda-forge`](https://conda-forge.org/) channel by default). + +Once `mamba` is installed, run +``` +mamba create -n sourmash_dev tox-conda rust git compilers pandoc +``` +to create an environment called `sourmash_dev` containing the programs needed +for development. + +To activate the new environment, run +``` +conda activate sourmash_dev +``` +and proceed to the ["Running tests and checks"](#running-tests-and-checks) section. + +### Using Nix + +Follow the [installation instructions](https://nixos.org/manual/nix/stable/#chap-installation) +for setting up Nix in your system (Linux or macOS). + +Once Nix is installed, run +``` +nix-shell +``` +to start an environment ready for [running tests and checks](#running-tests-and-checks). + +### General instructions + +As long as you have `tox` and a Rust compiler available, +you can skip `mamba` or `Nix`. + +For Rust, we suggest using `rustup` to install the Rust environment: +``` +curl https://sh.rustup.rs -sSf | sh +``` +And for `tox` you can run ``` python -m pip install tox ``` -and use `tox -l` to list available tasks. We suggest working on sourmash in a virtualenv; e.g. from within the -sourmash clone directory, you can do: +cloned repository (and after installing `tox` and Rust), you can do: ``` tox -e dev . .tox/dev/bin/activate ``` -You can run tests by invoking `make test` in the sourmash directory; -`tox -e py39` will run the Python tests with Python 3.9, -and `cargo test` will run the Rust tests. - -You can also explicitly install all the dependencies for sourmash by running +Finally, ou can also explicitly install all the Python dependencies for sourmash by running ``` pip install -r requirements.txt ``` +(but they are already installed in the virtualenv created with `tox -e dev`). -### If you're having trouble installing or using the development environment +## Running tests and checks -If you are getting an error that contains `ImportError: cannot import name 'to_bytes' from 'sourmash.minhash'`, then it's likely you need to update Rust and clean up your environment. Some installation issues can be solved by simply removing the intermediate build files with: +We use [`tox`](https://tox.readthedocs.io) for managing dependencies and +running tests and checks during development. +`tox -l` lists available tasks. -``` -make clean -``` +You can run tests by invoking `make test` in the sourmash directory; +`tox -e py39` will run the Python tests with Python 3.9, +and `cargo test` will run the Rust tests. ## Adding new changes @@ -99,7 +135,7 @@ A short description of the high-level files and dirs in the sourmash repo: ├── Makefile | Entry point for most development tasks ├── MANIFEST.in | Describes what files to add to the Python package ├── matplotlibrc | Configuration for matplotlib -├── nix.shell | Nix configuration for creating a dev environment +├── shell.nix | Nix configuration for creating a dev environment ├── paper.bib | References in the JOSS paper ├── paper.md | JOSS paper content ├── pyproject.toml | Python project definitions (build system and tooling) @@ -218,6 +254,18 @@ For the Rust core library we use `rMAJOR.MINOR.PATCH` The Rust version is not automated, and must be bumped in `src/core/Cargo.toml`. +## Common errors and solutions + +### Cannot import name `to_bytes` from `sourmash.minhash` + +If you are getting an error that contains `ImportError: cannot import name 'to_bytes' from 'sourmash.minhash'`, +then it's likely you need to update Rust and clean up your environment. +Some installation issues can be solved by simply removing the intermediate build files with: + +``` +make clean +``` + ## Contents ```{toctree} diff --git a/nix.shell b/nix.shell deleted file mode 100644 index 57ba5d1039..0000000000 --- a/nix.shell +++ /dev/null @@ -1,51 +0,0 @@ -let - moz_overlay = import (builtins.fetchTarball https://github.com/mozilla/nixpkgs-mozilla/archive/master.tar.gz); - nixpkgs = import { overlays = [ moz_overlay ]; }; - ruststable = (nixpkgs.latest.rustChannels.stable.rust); - - mach-nix = import ( - builtins.fetchGit { - url = "https://github.com/DavHau/mach-nix/"; - ref = "2.0.0"; - } - ); - - customPython = mach-nix.mkPython { - python = nixpkgs.python38; - requirements = '' - screed>=0.9 - cffi>=1.14.0 - numpy - matplotlib - scipy - deprecation>=2.0.6 - cachetools >=4,<5 - setuptools>=38.6.0 - milksnake - setuptools_scm>=3.2.0 - setuptools_scm_git_archive - pytest - pytest-cov - hypothesis - tox - ''; - }; - -in - with nixpkgs; - - nixpkgs.mkShell { - buildInputs = [ - customPython - git - stdenv - ruststable - stdenv.cc.cc.lib - ]; - - shellHook = '' - # workaround for https://github.com/NixOS/nixpkgs/blob/48dfc9fa97d762bce28cc8372a2dd3805d14c633/doc/languages-frameworks/python.section.md#python-setuppy-bdist_wheel-cannot-create-whl - export SOURCE_DATE_EPOCH=315532800 # 1980 - export LD_LIBRARY_PATH="${stdenv.cc.cc.lib}/lib64:$LD_LIBRARY_PATH"; - ''; -} diff --git a/nix/sources.json b/nix/sources.json new file mode 100644 index 0000000000..48de91396e --- /dev/null +++ b/nix/sources.json @@ -0,0 +1,38 @@ +{ + "niv": { + "branch": "master", + "description": "Easy dependency management for Nix projects", + "homepage": "https://github.com/nmattia/niv", + "owner": "nmattia", + "repo": "niv", + "rev": "3cd7914b2c4cff48927e11c216dadfab7d903fe5", + "sha256": "1agq4nvbhrylf2s77kb4xhh9k7xcwdwggq764k4jgsbs70py8cw3", + "type": "tarball", + "url": "https://github.com/nmattia/niv/archive/3cd7914b2c4cff48927e11c216dadfab7d903fe5.tar.gz", + "url_template": "https://github.com///archive/.tar.gz" + }, + "nixpkgs": { + "branch": "nixpkgs-unstable", + "description": "Nix Packages collection", + "homepage": "", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "d1f97a5eb5115289071d8449f26e7b92ce6b6709", + "sha256": "098rb747w4p5lxz74bj738bpjd4xjn0ahssjp9n8237dmmx3wg5p", + "type": "tarball", + "url": "https://github.com/NixOS/nixpkgs/archive/d1f97a5eb5115289071d8449f26e7b92ce6b6709.tar.gz", + "url_template": "https://github.com///archive/.tar.gz" + }, + "rust-overlay": { + "branch": "master", + "description": null, + "homepage": null, + "owner": "oxalica", + "repo": "rust-overlay", + "rev": "0bb9ef6d8b34e5579d7384201f3106a49ce3deca", + "sha256": "09cva1r53x1ihyzbssdscx4n5913pra8f006q095ww9wvfvz8bxf", + "type": "tarball", + "url": "https://github.com/oxalica/rust-overlay/archive/0bb9ef6d8b34e5579d7384201f3106a49ce3deca.tar.gz", + "url_template": "https://github.com///archive/.tar.gz" + } +} diff --git a/nix/sources.nix b/nix/sources.nix new file mode 100644 index 0000000000..1938409ddd --- /dev/null +++ b/nix/sources.nix @@ -0,0 +1,174 @@ +# This file has been generated by Niv. + +let + + # + # The fetchers. fetch_ fetches specs of type . + # + + fetch_file = pkgs: name: spec: + let + name' = sanitizeName name + "-src"; + in + if spec.builtin or true then + builtins_fetchurl { inherit (spec) url sha256; name = name'; } + else + pkgs.fetchurl { inherit (spec) url sha256; name = name'; }; + + fetch_tarball = pkgs: name: spec: + let + name' = sanitizeName name + "-src"; + in + if spec.builtin or true then + builtins_fetchTarball { name = name'; inherit (spec) url sha256; } + else + pkgs.fetchzip { name = name'; inherit (spec) url sha256; }; + + fetch_git = name: spec: + let + ref = + if spec ? ref then spec.ref else + if spec ? branch then "refs/heads/${spec.branch}" else + if spec ? tag then "refs/tags/${spec.tag}" else + abort "In git source '${name}': Please specify `ref`, `tag` or `branch`!"; + in + builtins.fetchGit { url = spec.repo; inherit (spec) rev; inherit ref; }; + + fetch_local = spec: spec.path; + + fetch_builtin-tarball = name: throw + ''[${name}] The niv type "builtin-tarball" is deprecated. You should instead use `builtin = true`. + $ niv modify ${name} -a type=tarball -a builtin=true''; + + fetch_builtin-url = name: throw + ''[${name}] The niv type "builtin-url" will soon be deprecated. You should instead use `builtin = true`. + $ niv modify ${name} -a type=file -a builtin=true''; + + # + # Various helpers + # + + # https://github.com/NixOS/nixpkgs/pull/83241/files#diff-c6f540a4f3bfa4b0e8b6bafd4cd54e8bR695 + sanitizeName = name: + ( + concatMapStrings (s: if builtins.isList s then "-" else s) + ( + builtins.split "[^[:alnum:]+._?=-]+" + ((x: builtins.elemAt (builtins.match "\\.*(.*)" x) 0) name) + ) + ); + + # The set of packages used when specs are fetched using non-builtins. + mkPkgs = sources: system: + let + sourcesNixpkgs = + import (builtins_fetchTarball { inherit (sources.nixpkgs) url sha256; }) { inherit system; }; + hasNixpkgsPath = builtins.any (x: x.prefix == "nixpkgs") builtins.nixPath; + hasThisAsNixpkgsPath = == ./.; + in + if builtins.hasAttr "nixpkgs" sources + then sourcesNixpkgs + else if hasNixpkgsPath && ! hasThisAsNixpkgsPath then + import {} + else + abort + '' + Please specify either (through -I or NIX_PATH=nixpkgs=...) or + add a package called "nixpkgs" to your sources.json. + ''; + + # The actual fetching function. + fetch = pkgs: name: spec: + + if ! builtins.hasAttr "type" spec then + abort "ERROR: niv spec ${name} does not have a 'type' attribute" + else if spec.type == "file" then fetch_file pkgs name spec + else if spec.type == "tarball" then fetch_tarball pkgs name spec + else if spec.type == "git" then fetch_git name spec + else if spec.type == "local" then fetch_local spec + else if spec.type == "builtin-tarball" then fetch_builtin-tarball name + else if spec.type == "builtin-url" then fetch_builtin-url name + else + abort "ERROR: niv spec ${name} has unknown type ${builtins.toJSON spec.type}"; + + # If the environment variable NIV_OVERRIDE_${name} is set, then use + # the path directly as opposed to the fetched source. + replace = name: drv: + let + saneName = stringAsChars (c: if isNull (builtins.match "[a-zA-Z0-9]" c) then "_" else c) name; + ersatz = builtins.getEnv "NIV_OVERRIDE_${saneName}"; + in + if ersatz == "" then drv else + # this turns the string into an actual Nix path (for both absolute and + # relative paths) + if builtins.substring 0 1 ersatz == "/" then /. + ersatz else /. + builtins.getEnv "PWD" + "/${ersatz}"; + + # Ports of functions for older nix versions + + # a Nix version of mapAttrs if the built-in doesn't exist + mapAttrs = builtins.mapAttrs or ( + f: set: with builtins; + listToAttrs (map (attr: { name = attr; value = f attr set.${attr}; }) (attrNames set)) + ); + + # https://github.com/NixOS/nixpkgs/blob/0258808f5744ca980b9a1f24fe0b1e6f0fecee9c/lib/lists.nix#L295 + range = first: last: if first > last then [] else builtins.genList (n: first + n) (last - first + 1); + + # https://github.com/NixOS/nixpkgs/blob/0258808f5744ca980b9a1f24fe0b1e6f0fecee9c/lib/strings.nix#L257 + stringToCharacters = s: map (p: builtins.substring p 1 s) (range 0 (builtins.stringLength s - 1)); + + # https://github.com/NixOS/nixpkgs/blob/0258808f5744ca980b9a1f24fe0b1e6f0fecee9c/lib/strings.nix#L269 + stringAsChars = f: s: concatStrings (map f (stringToCharacters s)); + concatMapStrings = f: list: concatStrings (map f list); + concatStrings = builtins.concatStringsSep ""; + + # https://github.com/NixOS/nixpkgs/blob/8a9f58a375c401b96da862d969f66429def1d118/lib/attrsets.nix#L331 + optionalAttrs = cond: as: if cond then as else {}; + + # fetchTarball version that is compatible between all the versions of Nix + builtins_fetchTarball = { url, name ? null, sha256 }@attrs: + let + inherit (builtins) lessThan nixVersion fetchTarball; + in + if lessThan nixVersion "1.12" then + fetchTarball ({ inherit url; } // (optionalAttrs (!isNull name) { inherit name; })) + else + fetchTarball attrs; + + # fetchurl version that is compatible between all the versions of Nix + builtins_fetchurl = { url, name ? null, sha256 }@attrs: + let + inherit (builtins) lessThan nixVersion fetchurl; + in + if lessThan nixVersion "1.12" then + fetchurl ({ inherit url; } // (optionalAttrs (!isNull name) { inherit name; })) + else + fetchurl attrs; + + # Create the final "sources" from the config + mkSources = config: + mapAttrs ( + name: spec: + if builtins.hasAttr "outPath" spec + then abort + "The values in sources.json should not have an 'outPath' attribute" + else + spec // { outPath = replace name (fetch config.pkgs name spec); } + ) config.sources; + + # The "config" used by the fetchers + mkConfig = + { sourcesFile ? if builtins.pathExists ./sources.json then ./sources.json else null + , sources ? if isNull sourcesFile then {} else builtins.fromJSON (builtins.readFile sourcesFile) + , system ? builtins.currentSystem + , pkgs ? mkPkgs sources system + }: rec { + # The sources, i.e. the attribute set of spec name to spec + inherit sources; + + # The "pkgs" (evaluated nixpkgs) to use for e.g. non-builtin fetchers + inherit pkgs; + }; + +in +mkSources (mkConfig {}) // { __functor = _: settings: mkSources (mkConfig settings); } diff --git a/shell.nix b/shell.nix new file mode 100644 index 0000000000..0ac3e4b47c --- /dev/null +++ b/shell.nix @@ -0,0 +1,22 @@ +let + sources = import ./nix/sources.nix; + pkgs = import sources.nixpkgs { overlays = [ (import sources.rust-overlay) ]; }; +in + with pkgs; + + pkgs.mkShell { + buildInputs = [ + rust-bin.stable.latest.rust + git + stdenv.cc.cc.lib + (python38.withPackages(ps: with ps; [ virtualenv tox setuptools ])) + (python39.withPackages(ps: with ps; [ virtualenv setuptools ])) + (python37.withPackages(ps: with ps; [ virtualenv setuptools ])) + ]; + + shellHook = '' + # workaround for https://github.com/NixOS/nixpkgs/blob/48dfc9fa97d762bce28cc8372a2dd3805d14c633/doc/languages-frameworks/python.section.md#python-setuppy-bdist_wheel-cannot-create-whl + export SOURCE_DATE_EPOCH=315532800 # 1980 + export LD_LIBRARY_PATH="${stdenv.cc.cc.lib}/lib64:$LD_LIBRARY_PATH"; + ''; + }