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

Add command line tool to Nixpkgs? #169

Open
doronbehar opened this issue Oct 28, 2020 · 13 comments
Open

Add command line tool to Nixpkgs? #169

doronbehar opened this issue Oct 28, 2020 · 13 comments

Comments

@doronbehar
Copy link

Using nixUnstable, I tried installing mach-nix with:

nix registry add mach github:DavHau/mach-nix
nix search mach

And I encountered:

warning: --- Large path ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- nix
dumping very large path (> 256 MiB); this may run out of memory
warning: --- Large path --- nix-daemon
dumping very large path (> 256 MiB); this may run out of memory
[1 copied (75.7 MiB), 150.2/54.5 MiB DL] downloading 'https://github.com/DavHau/nix-pypi-fetcher/tarball/e105186d0101ead100a64e86b1cd62abd1482error: --- EvalError --------------------------------------------------------------------------------------------------------------------- nix
at: (46:24) in file: /nix/store/jndn6xr4zk84wy746qg3kcajf7clg1h0-nix-pypi-fetcher-src/default.nix

    45|   '';
    46|   allNames = fromJSON (readFile allNamesJsonFile);
      |                        ^
    47| in

cannot read '/nix/store/q3v00s260vb7c5b8k95jxbzhhrhjss7r-all-package-names', since path '/nix/store/vpxwskniiixv133r82rhqdj8rk1g096i-all-package-names.drv' is not valid
(use '--show-trace' to show detailed location information)

So I guess https://github.com/DavHau/nix-pypi-fetcher/blob/master/default.nix should be patched, but speaking of the mach-nix tool, which is what I wanted to obtain, would it be a good idea adding it to Nixpkgs? For more casual Nix users?

@DavHau
Copy link
Owner

DavHau commented Oct 28, 2020

Strange, I don't get the error when I execute the same commands. And I'm not quite sure what exactly triggers the error. Any idea?

I have thought about adding it to nixpkgs a few times, but I was always afraid that the nixpkgs mach-nix version would become outdated rather quickly since there is still a lot of movement going on. What do you think?

For having a kind of long term stable release of mach-nix residing in nixpkgs, it would probably be a good idea to add some functionality to the cmdline tool to update the pypi db version easily. Otherwise people might be stuck on an outdated pypi snapshot for several months.

@doronbehar
Copy link
Author

I have thought about adding it to nixpkgs a few times, but I was always afraid that the nixpkgs mach-nix version would become outdated rather quickly since there is still a lot of movement going on. What do you think?

For having a kind of long term stable release of mach-nix residing in nixpkgs, it would probably be a good idea to add some functionality to the cmdline tool to update the pypi db version easily. Otherwise people might be stuck on an outdated pypi snapshot for several months.

Considering the fact that the command line tool is binded to a specific revision of the pypi nix db, I guess it's not worth it to put it in Nixpkgs..

Strange, I don't get the error when I execute the same commands. And I'm not quite sure what exactly triggers the error. Any idea?

I think this is the issue:

DavHau/nix-pypi-fetcher#5

@DavHau
Copy link
Owner

DavHau commented Oct 29, 2020

The code style there might not be good and should be improved, but I don't see any impurities there. If we're both using linux we should get the same result. If not there there must be a problem somewhere else. Or am I confusing something?

doronbehar added a commit to doronbehar/nix-pypi-fetcher that referenced this issue Oct 29, 2020
@doronbehar
Copy link
Author

There are a number of issues. It might be a bit hard to solve this in a nixStable compatible way, but I tried:

diff --git i/flake.lock w/flake.lock
index 589d55e..fc3a276 100644
--- i/flake.lock
+++ w/flake.lock
@@ -17,11 +17,11 @@
     },
     "nixpkgs": {
       "locked": {
-        "lastModified": 1601171649,
-        "narHash": "sha256-G3RUAi2DUq6r3ntASLS+LZC/Eamot55W1+xmBOgEh3M=",
+        "lastModified": 1603758576,
+        "narHash": "sha256-xKFmWVx+rFInBao3gtmvXcd0LjHD1+0c1Ef5PJDrbuM=",
         "owner": "NixOS",
         "repo": "nixpkgs",
-        "rev": "84d74ae9c9cbed73274b8e4e00be14688ffc93fe",
+        "rev": "1dc37370c489b610f8b91d7fdd40633163ffbafd",
         "type": "github"
       },
       "original": {
diff --git i/flake.nix w/flake.nix
index 2b4b98a..9b0427f 100644
--- i/flake.nix
+++ w/flake.nix
@@ -30,4 +30,4 @@
         defaultApp = { type = "app"; program = "${defaultPackage."${system}"}/bin/mach-nix"; };
       }
   );
-}
\ No newline at end of file
+}
diff --git i/lib/extractor/default.nix w/lib/extractor/default.nix
index f10a41e..9046f34 100644
--- i/lib/extractor/default.nix
+++ w/lib/extractor/default.nix
@@ -1,11 +1,11 @@
 let
   pkgs = import <nixpkgs> { config = { allowUnfree = true; }; overlays = []; };
-  commit = "1434cc0ee2da462f0c719d3a4c0ab4c87d0931e7";
+  commit = "5798bc0422ca639f2f0ed6fbb22827fc7c8f794c";
   fetchPypiSrc = builtins.fetchTarball {
    name = "nix-pypi-fetcher";
-   url = "https://github.com/DavHau/nix-pypi-fetcher/archive/${commit}.tar.gz";
+   url = "https://github.com/doronbehar/nix-pypi-fetcher/archive/${commit}.tar.gz";
    # Hash obtained using `nix-prefetch-url --unpack <url>`
-   sha256 = "080l189zzwrv75jgr7agvs4hjv4i613j86d4qky154fw5ncp0mnp";
+   sha256 = "1fyj5gdgmd9msfmsfx2r040hjsyncm99d178rsf5kfmgd9kmkvmg";
   };
   fetchPypi = import (fetchPypiSrc);
   patchDistutils = python_env:

But then I noticed it won't use the updated fork due to:

pypi_fetcher_sha256 = removeSuffix "\n" (readFile "${deps_db_src}/PYPI_FETCHER_SHA256");
pypi_fetcher_src = fetchTarball {
name = "nix-pypi-fetcher-src";
url = "https://github.com/DavHau/nix-pypi-fetcher/tarball/${pypi_fetcher_commit}";
sha256 = "${pypi_fetcher_sha256}";
};

I think it'd be best to solve this by using pypi-fetcher as a flakes input. For nixStable compatibility, I'd tryto give it a default value (if not given from flake.nix) in default.nix which is called by flake.nix. Using both a python script to update a depdendency and it's hash, a comment saying: "Use nix-prefetch-url to update this hash" And a JSON file is prone to issues.. There should be 1 place where the commit and hash and url are defined, period.

For nix-pypi-fetcher to become a flakes input, it must be a flake in itself, which would probably worth doing with a local git checkout of the repo, which I am currently reluctant to doing (since it's so heavy).

@DavHau
Copy link
Owner

DavHau commented Oct 30, 2020

The nix-pypi-fetcher is hardcoded via pypi-deps-db. And this is intended. It doesn't make any sense to update the nix-pypi-fetcher if the dependency DB doesn't know those dependencies. They wouldn't be used in that case. Those two project are basically locked to each other. But you can of course define pypi-deps-db, which will then in turn also come with a new nix-pypi-fetcher.

The dependencies which can be defined are:

  • nixpkgs
  • pypi-deps-db
  • conda channels (coming soon)

I agree that the whole dependency management should be done via flakes, with some downward compatible mechanism to work with older nix versions. Probably via https://github.com/edolstra/flake-compat ?

I myself didn't put much effort into the flakes stuff yet. Right now I'm busy working in the conda support. I'd welcome any improvements on that.

@DavHau
Copy link
Owner

DavHau commented Oct 30, 2020

The nix-pypi-fetcher version hardcoded in ./lib/extractor/default.nix is irelevent. This is only used for testing. In normal operation the extractor doesn't need the nix-pypi-fetcher.
But of course the impurities in the extractor caused by <nixpkgs> should be removed, like already mentioned by #153 (comment)

@doronbehar
Copy link
Author

I agree that the whole dependency management should be done via flakes, with some downward compatible mechanism to work with older nix versions. Probably via edolstra/flake-compat ?

Yea that sounds like an even better idea to me.

Overall, I'm getting a bit lost in how all of these different repos are connected, and I'd just like to find out if DavHau/nix-pypi-fetcher#5 is the cause of the original issue. So, how would I go about and dirtily use doronbehar/nix-pypi-fetcher@5798bc0 as the new source of the nix-pypi-fetcher?

doronbehar added a commit to doronbehar/nix-pypi-fetcher that referenced this issue Oct 30, 2020
@doronbehar
Copy link
Author

This is also bad:

deps_db_src = fetchTarball {
name = "pypi-deps-db-src";
url = "https://github.com/DavHau/pypi-deps-db/tarball/${pypi_deps_db_commit}";
sha256 = "${pypi_deps_db_sha256}";
};
pypi_fetcher_commit = removeSuffix "\n" (readFile "${deps_db_src}/PYPI_FETCHER_COMMIT");
pypi_fetcher_sha256 = removeSuffix "\n" (readFile "${deps_db_src}/PYPI_FETCHER_SHA256");
pypi_fetcher_src = fetchTarball {
name = "nix-pypi-fetcher-src";
url = "https://github.com/DavHau/nix-pypi-fetcher/tarball/${pypi_fetcher_commit}";
sha256 = "${pypi_fetcher_sha256}";
};

You have 3 different places where the pypi-fetcher commit is grabbed from. We are lucky we have flakes... Never mind that. Indeed the issue is not about with expr;. I managed to get this diff:

diff --git i/flake.lock w/flake.lock
index 589d55e..fc3a276 100644
--- i/flake.lock
+++ w/flake.lock
@@ -17,11 +17,11 @@
     },
     "nixpkgs": {
       "locked": {
-        "lastModified": 1601171649,
-        "narHash": "sha256-G3RUAi2DUq6r3ntASLS+LZC/Eamot55W1+xmBOgEh3M=",
+        "lastModified": 1603758576,
+        "narHash": "sha256-xKFmWVx+rFInBao3gtmvXcd0LjHD1+0c1Ef5PJDrbuM=",
         "owner": "NixOS",
         "repo": "nixpkgs",
-        "rev": "84d74ae9c9cbed73274b8e4e00be14688ffc93fe",
+        "rev": "1dc37370c489b610f8b91d7fdd40633163ffbafd",
         "type": "github"
       },
       "original": {
diff --git i/lib/extractor/default.nix w/lib/extractor/default.nix
index f10a41e..9046f34 100644
--- i/lib/extractor/default.nix
+++ w/lib/extractor/default.nix
@@ -1,11 +1,11 @@
 let
   pkgs = import <nixpkgs> { config = { allowUnfree = true; }; overlays = []; };
-  commit = "1434cc0ee2da462f0c719d3a4c0ab4c87d0931e7";
+  commit = "5798bc0422ca639f2f0ed6fbb22827fc7c8f794c";
   fetchPypiSrc = builtins.fetchTarball {
    name = "nix-pypi-fetcher";
-   url = "https://github.com/DavHau/nix-pypi-fetcher/archive/${commit}.tar.gz";
+   url = "https://github.com/doronbehar/nix-pypi-fetcher/archive/${commit}.tar.gz";
    # Hash obtained using `nix-prefetch-url --unpack <url>`
-   sha256 = "080l189zzwrv75jgr7agvs4hjv4i613j86d4qky154fw5ncp0mnp";
+   sha256 = "1fyj5gdgmd9msfmsfx2r040hjsyncm99d178rsf5kfmgd9kmkvmg";
   };
   fetchPypi = import (fetchPypiSrc);
   patchDistutils = python_env:
diff --git i/mach_nix/nix/deps-db-and-fetcher.nix w/mach_nix/nix/deps-db-and-fetcher.nix
index f9f3875..9bdb057 100644
--- i/mach_nix/nix/deps-db-and-fetcher.nix
+++ w/mach_nix/nix/deps-db-and-fetcher.nix
@@ -10,15 +10,15 @@ let
     url = "https://github.com/DavHau/pypi-deps-db/tarball/${pypi_deps_db_commit}";
     sha256 = "${pypi_deps_db_sha256}";
   };
-  pypi_fetcher_commit = removeSuffix "\n" (readFile "${deps_db_src}/PYPI_FETCHER_COMMIT");
-  pypi_fetcher_sha256 = removeSuffix "\n" (readFile "${deps_db_src}/PYPI_FETCHER_SHA256");
+  pypi_fetcher_commit = "8250decda817133577d401eb38cba78a378dd3af";
+  # pypi_fetcher_sha256 = removeSuffix "\n" (readFile "${deps_db_src}/PYPI_FETCHER_SHA256");
   pypi_fetcher_src = fetchTarball {
-    name = "nix-pypi-fetcher-src";
-    url = "https://github.com/DavHau/nix-pypi-fetcher/tarball/${pypi_fetcher_commit}";
-    sha256 = "${pypi_fetcher_sha256}";
+    name = "nix-pypi-fetcher";
+    url = "https://github.com/doronbehar/nix-pypi-fetcher/archive/${pypi_fetcher_commit}.tar.gz";
   };
   pypi_fetcher = import pypi_fetcher_src {
     inherit pkgs;
+    inherit lib;
   };
 in
 let

And with nix search --impure --update-input nixpkgs . I got the same error as before.. I completely misinterpreted the situation.

I suspect that builtins.readFile acts differently upon allNamesJsonFile if it's created with nixUnstable compared to other Nix (What is your Nix version?)

That json file shouldn't be a json file - it could be evaluated all in pure Nix, as it seems from that python snippet - buckets could be a Nix variable, and all_names is just builtins.catAttrs.

@DavHau
Copy link
Owner

DavHau commented Oct 30, 2020

Hey, nice to see what you figured out so far!

If I recall correctly, the only reason why i retrieve the list of all names via builder, is because this is a quite heavy operation and would slow down nix evaluation. by doing this inside a builder allows the result to be cached, so the evaluation will be fast the second time.

BTW, i don't really understand what the error since path '/nix/store/vpxwskniiixv133r82rhqdj8rk1g096i-all-package-names.drv' is not valid means.

How can a drv not be vaild? Maybe the nix source code of this error reveals a bit more info.

My nix version is: 3.0pre20200829_f156513

@doronbehar
Copy link
Author

Nix issue: NixOS/nix#4210

@DavHau
Copy link
Owner

DavHau commented Nov 29, 2020

Since 3.1.1, the CLI has a version management. It also manages the version of mach-nix itself. Not yet via flakes, but it souldn't be difficult to switch to a pure flakes management as soon as nixFlakes becomes stable.

I now could even change the CLI to always initialize with the newest mach-nix version whenever mach-nix env ... is executed.
That way it wouldn't matter if an outdated version is used to execute that command.

In flakes terms, the CLI could simply be a wrapper around nix run github:DavHau/mach-nix/stable or something like that.

@DavHau
Copy link
Owner

DavHau commented Dec 2, 2020

Mach-nix just got merged into the flakes registry. Now it can be executed via:

nix run mach-nix

I guess that solves the problem at least for nixFlakes users.

@doronbehar
Copy link
Author

Now it can be executed via:

nix run mach-nix

I guess that solves the problem at least for nixFlakes users.

OK That's nice, but it'd be nice if in general it was easy to override the sources and their hashes used in this repo. Thanks for the update though :).

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

No branches or pull requests

2 participants