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

emacs updater: trivialBuild -> melpaBuild #329008

Merged
merged 2 commits into from
Jul 28, 2024
Merged

emacs updater: trivialBuild -> melpaBuild #329008

merged 2 commits into from
Jul 28, 2024

Conversation

AndersonTorres
Copy link
Member

This can cause some problems...

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: emacs Text editor label Jul 21, 2024
Copy link
Contributor

@jian-lin jian-lin left a comment

Choose a reason for hiding this comment

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

I am slightly against this change because I do not see much benefit it brings and it add "noise" to git blame.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jul 21, 2024
@AndersonTorres
Copy link
Member Author

AndersonTorres commented Jul 22, 2024

I am slightly against this change because I do not see much benefit it brings and it add "noise" to git blame.

Well, it is a good thing to be consistent.
We should use the tools we recommend to the community, even more given that these tools were improved for that same purpose.

@jian-lin
Copy link
Contributor

jian-lin commented Jul 22, 2024

We should use the tools we recommend to the community

melpaBuild is recommended for most cases but not all. There are cases where trivialBuild is better. There are cases where trivialBuild is equally good, such as here. There are also cases where elpaBuild is equally good.

it is a good thing to be consistent.

Consistence without cost is good. Consistence at the cost of more noisy git history is a judgement call, I guess. If changes for version and hash are factored out into different commits and changes for src.repo, src.owner and moving lines are removed, the git history is less noisy. Then I may support this change.

@AndersonTorres
Copy link
Member Author

Consistence at the cost of more noisy git history is a judgement call, I guess.

If I remove the commit that changed the repos to emacsmirror, the patch will change less than 20 lines.
Further, technically the hashes were not "changed", they were merely rewritten in SRI format.

Since the original repos are still there, I am fine with removing the mirror.

If changes for version and hash are factored out into different commits

Done :)

@jian-lin
Copy link
Contributor

If changes for version and hash are factored out into different commits

Done :)

I still only see one commit in this PR.

@jian-lin
Copy link
Contributor

Since it is an important part for updating MELPA packages, tests must be done before this can be merged. I may not be able to do the test because my network is not very good.

@AndersonTorres
Copy link
Member Author

I still only see one commit in this PR.

My network delayed. Apologies for the technical failure.

Since it is an important part for updating MELPA packages, tests must be done before this can be merged. I may not be able to do the test because my network is not very good.

I am not sure, but if "run the update scripts inside elisp-packages" can be regarded as a test, I believe I can do it.

The only problem is that, as far as I know, the scripts fail in my current flake setup.

I am doing some tests in my spare notebook, installing NixOS from scratch without flakes.

@jian-lin
Copy link
Contributor

Could you provide the error and reproducible steps for your error? I do not think a new installable of NixOS is necessary.

@AndersonTorres
Copy link
Member Author

AndersonTorres commented Jul 22, 2024

Full log (click to expand)
$> master/pkgs/applications/editors/emacs/elisp-packages >$
./update-elpa

error:
       … while calling the 'import' builtin
         at «string»:1:2:
            1| (import <nixpkgs> {}).bashInteractive
             |  ^

       … while realising the context of a path

       … while calling the 'findFile' builtin
         at «string»:1:9:
            1| (import <nixpkgs> {}).bashInteractive
             |         ^

       error: file 'nixpkgs' was not found in the Nix search path (add it using $NIX_PATH or -I)
will use bash from your environment
this derivation will be built:
  /nix/store/n8z3cvgmq3pwzg5b46908jpgvdcgjrla-source.drv
building '/nix/store/n8z3cvgmq3pwzg5b46908jpgvdcgjrla-source.drv'...
sourcing setup hook '/nix/store/yq6n8b0mnk0qxzbs3ajsjcp8ziwqylrl-patchelf-0.15.0/nix-support/setup-hook'
sourcing setup hook '/nix/store/iks1pihvbilsh5sy8qvpd638k422w9i8-update-autotools-gnu-config-scripts-hook/nix-support/setup-hook'
sourcing setup hook '/nix/store/h9lc1dpi14z7is86ffhl3ld569138595-audit-tmpdir.sh'
sourcing setup hook '/nix/store/m54bmrhj6fqz8nds5zcj97w9s9bckc9v-compress-man-pages.sh'
sourcing setup hook '/nix/store/wgrbkkaldkrlrni33ccvm3b6vbxzb656-make-symlinks-relative.sh'
sourcing setup hook '/nix/store/5yzw0vhkyszf2d179m0qfkgxmp5wjjx4-move-docs.sh'
sourcing setup hook '/nix/store/fyaryjvghbkpfnsyw97hb3lyb37s1pd6-move-lib64.sh'
sourcing setup hook '/nix/store/kd4xwxjpjxi71jkm6ka0np72if9rm3y0-move-sbin.sh'
sourcing setup hook '/nix/store/pag6l61paj1dc9sv15l7bm5c17xn5kyk-move-systemd-user-units.sh'
sourcing setup hook '/nix/store/jivxp510zxakaaic7qkrb7v1dd2rdbw9-multiple-outputs.sh'
sourcing setup hook '/nix/store/ilaf1w22bxi6jsi45alhmvvdgy4ly3zs-patch-shebangs.sh'
sourcing setup hook '/nix/store/cickvswrvann041nqxb0rxilc46svw1n-prune-libtool-files.sh'
sourcing setup hook '/nix/store/xyff06pkhki3qy1ls77w10s0v79c9il0-reproducible-builds.sh'
sourcing setup hook '/nix/store/ngg1cv31c8c7bcm2n8ww4g06nq7s4zhm-set-source-date-epoch-to-latest.sh'
sourcing setup hook '/nix/store/gps9qrh99j7g02840wv5x78ykmz30byp-strip.sh'
exporting https://github.com/nix-community/emacs2nix.git (rev e5389c3d7be9c3af135f022d86c61767d41c364f) into /nix/store/xyb2gnlj8rivpknig8w0dmssh7rhnvn3-source
Initialized empty Git repository in /nix/store/xyb2gnlj8rivpknig8w0dmssh7rhnvn3-source/.git/
remote: Enumerating objects: 62, done.
remote: Counting objects: 100% (62/62), done.
remote: Compressing objects: 100% (57/57), done.
remote: Total 62 (delta 14), reused 27 (delta 1), pack-reused 0
Unpacking objects: 100% (62/62), 50.42 KiB | 1.94 MiB/s, done.
From https://github.com/nix-community/emacs2nix
 * branch            e5389c3d7be9c3af135f022d86c61767d41c364f -> FETCH_HEAD
Switched to a new branch 'fetchgit'
Submodule 'hnix' (https://github.com/jwiegley/hnix.git) registered for path 'hnix'
Cloning into '/nix/store/xyb2gnlj8rivpknig8w0dmssh7rhnvn3-source/hnix'...
remote: Enumerating objects: 412, done.        
remote: Counting objects: 100% (412/412), done.        
remote: Compressing objects: 100% (282/282), done.        
remote: Total 412 (delta 9), reused 302 (delta 5), pack-reused 0        
Receiving objects: 100% (412/412), 387.14 KiB | 941.00 KiB/s, done.
Resolving deltas: 100% (9/9), done.
remote: Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
remote: Enumerating objects: 142, done.
remote: Counting objects: 100% (142/142), done.
remote: Compressing objects: 100% (73/73), done.
remote: Total 77 (delta 12), reused 20 (delta 0), pack-reused 0
Unpacking objects: 100% (77/77), 108.01 KiB | 4.91 MiB/s, done.
From https://github.com/jwiegley/hnix
 * branch            2828d32144acf5c6632f4fa088279b3a51032ea1 -> FETCH_HEAD
Submodule path 'hnix': checked out '2828d32144acf5c6632f4fa088279b3a51032ea1'
Submodule 'data/nix' (https://github.com/haskell-nix/nix) registered for path 'hnix/data/nix'
Cloning into '/nix/store/xyb2gnlj8rivpknig8w0dmssh7rhnvn3-source/hnix/data/nix'...
remote: Enumerating objects: 1056, done.        
remote: Counting objects: 100% (1056/1056), done.        
remote: Compressing objects: 100% (931/931), done.        
remote: Total 1056 (delta 18), reused 479 (delta 5), pack-reused 0        
Receiving objects: 100% (1056/1056), 1.30 MiB | 3.82 MiB/s, done.
Resolving deltas: 100% (18/18), done.
remote: Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
remote: Enumerating objects: 779, done.
remote: Counting objects: 100% (779/779), done.
remote: Compressing objects: 100% (458/458), done.
remote: Total 485 (delta 244), reused 86 (delta 13), pack-reused 0
Receiving objects: 100% (485/485), 521.94 KiB | 2.01 MiB/s, done.
Resolving deltas: 100% (244/244), completed with 223 local objects.
From https://github.com/haskell-nix/nix
 * branch            c43033101cbba5f23e8b1165521a8a6e1eb082ea -> FETCH_HEAD
Submodule path 'hnix/data/nix': checked out 'c43033101cbba5f23e8b1165521a8a6e1eb082ea'
removing `.git'...
error:
       … while calling the 'import' builtin
         at /nix/store/xyb2gnlj8rivpknig8w0dmssh7rhnvn3-source/shell-fetch.nix:3:13:
            2|   emacs2nix = import ./default.nix;
            3|   nixpkgs = import <nixpkgs> {};
             |             ^
            4|

       … while realising the context of a path

       … while calling the 'findFile' builtin
         at /nix/store/xyb2gnlj8rivpknig8w0dmssh7rhnvn3-source/shell-fetch.nix:3:20:
            2|   emacs2nix = import ./default.nix;
            3|   nixpkgs = import <nixpkgs> {};
             |                    ^
            4|

       error: file 'nixpkgs' was not found in the Nix search path (add it using $NIX_PATH or -I)

If it is useful, my setup is on Bitbucket: https://bitbucket.org/anderson_torres/etc/src/trunk/

@jian-lin
Copy link
Contributor

Looks like you do not have NIX_PATH env. You can set that like this:

export NIX_PATH=nixpkgs=/path/to/your/nixpkgs
./update-elpa

or

# flake:nixpkgs means nixpkgs in your flake registry
export NIX_PATH=nixpkgs=flake:nixpkgs
./update-elpa

@jian-lin
Copy link
Contributor

If it is useful, my setup is on Bitbucket: https://bitbucket.org/anderson_torres/etc/src/trunk/

Oh, please delete nix.channel.enable = false. It's a known issue

@jian-lin
Copy link
Contributor

I find that the built result is the same on my system using diff, so presumably this change should be pretty safe.

# default.nix
let
  pkgs = import <nixpkgs> { };
  epkgs = pkgs.emacs.pkgs;
  # builder = pkgs.emacs.pkgs.trivialBuild;
  builder = epkgs.melpaBuild;
in
rec {
  promise = builder {
    pname = "promise";
    version = "1";
    src = pkgs.fetchFromGitHub {
      owner = "bendlas";
      repo = "emacs-promise";
      rev = "4da97087c5babbd8429b5ce62a8323b9b03c6022";
      sha256 = "0yin7kj69g4zxs30pvk47cnfygxlaw7jc7chr3b36lz51yqczjsy";
    };
    # add missing dependency async
    packageRequires = [ epkgs.async ];
    postPatch = ''
      substituteInPlace promise.el \
        --replace-fail "(require 'promise-done)" "(require 'promise-done) (require 'async)"
    '';
  };  

  semaphore = builder {
    pname = "semaphore";
    version = "1";
    packageRequires = [ promise ];
    src = pkgs.fetchFromGitHub {
      owner = "webnf";
      repo = "semaphore.el";
      rev = "93802cb093073bc6a6ccd797328dafffcef248e0";
      sha256 = "09pfyp27m35sv340xarhld7xx2vv5fs5xj4418709iw6l6hpk853";
    };
  };
}
nix build --include nixpkgs-system=/path/to/nixpkgs --file .

@AndersonTorres
Copy link
Member Author

Now I am running the scritps; the melpa one is stuck at cmake

@jian-lin
Copy link
Contributor

jian-lin commented Jul 27, 2024

updater-emacs.nix is only used when updating MELPA (unstable and stable). So to test this PR, only running update-melpa is enough.

Or you do not even need to run anything to test, if the built result of promise and semaphore is the same bit-by-bit.

@AndersonTorres
Copy link
Member Author

When I used el-get, I remember that cmake-mode was very slow since it downloaded the whole cmake project just to pick one file...

Does it occur in the elisp overlay too?

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 501+ 10.rebuild-darwin: 501-1000 10.rebuild-linux: 501+ 10.rebuild-linux: 501-1000 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jul 27, 2024
@AndersonTorres
Copy link
Member Author

AndersonTorres commented Jul 27, 2024

Here is a dump of strace when running update-melpa.

Copy link
Contributor

@jian-lin jian-lin left a comment

Choose a reason for hiding this comment

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

Please remove those bump commits and add the missing dependency. Then I think it is in good shape.

@AndersonTorres AndersonTorres marked this pull request as ready for review July 28, 2024 13:13
@jian-lin
Copy link
Contributor

I have verified that with this diff, the build results are the same bit-by-bit.

--- a/pkgs/applications/editors/emacs/elisp-packages/updater-emacs.nix
+++ b/pkgs/applications/editors/emacs/elisp-packages/updater-emacs.nix
@@ -3,9 +3,10 @@ let
 
   emacsEnv = pkgs.emacs.pkgs.withPackages (epkgs: let
 
-    promise = epkgs.trivialBuild {
+    promise = epkgs.melpaBuild {
       pname = "promise";
       version = "1";
+      packageRequires = [ epkgs.async ];
       src = pkgs.fetchFromGitHub {
         owner = "bendlas";
         repo = "emacs-promise";
@@ -14,7 +15,7 @@ let
       };
     };
 
-    semaphore = epkgs.trivialBuild {
+    semaphore = epkgs.melpaBuild {
       pname = "semaphore";
       version = "1";
       packageRequires = [ promise ];

Weird, I think I have posted this.

@jian-lin jian-lin merged commit 6badbad into NixOS:master Jul 28, 2024
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: emacs Text editor 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants