NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.39k stars 14.34k forks source link

Tracking issue: trivialBuild should retire in favor of melpaBuild #278925

Open AndersonTorres opened 11 months ago

AndersonTorres commented 11 months ago

Describe the bug

According to @jian-lin the trivialBuild function has some problems that make it a bad fit for Emacs ecosystem in Nixpkgs.

In packages made via trivialBuild:

References:


Add a :+1: reaction to issues you find important.

jian-lin commented 11 months ago

I think we should:

AndersonTorres commented 11 months ago

We should do this in reverse: first by looking at current code that uses trivialBuild, and only then to warn new devs.

I say this because I remember some packages fail with melpaBuild. I didn't know how to use it with sunrise-commander

AndersonTorres commented 11 months ago

Directories inside pkgs/applications/editors/emacs/elisp-packages/manual-packages/ that call trivialBuild:

EDIT: deleted because obsoleted.

AndersonTorres commented 11 months ago

Example: I have tried to update sunrise-commander:

{ lib
, elpaBuild
, fetchFromGitHub
, emacs
}:

elpaBuild {
  pname = "sunrise-commander";
  ename = "sunrise-commander";
  version = "unstable-2021-09-27";

  src = fetchFromGitHub {
    owner = "sunrise-commander";
    repo = "sunrise-commander";
    rev = "16e6df7e86c7a383fb4400fae94af32baf9cb24e";
    hash = "sha256-D36qiRi5OTZrBtJ/bD/javAWizZ8NLlC/YP4rdLCSsw=";
  };

  packageRequires = [
    emacs
  ];

  meta = {
    homepage = "https://github.com/sunrise-commander/sunrise-commander/";
    description = "Orthodox (two-pane) file manager for Emacs";
    license = lib.licenses.gpl3Plus;
    maintainers = [ lib.maintainers.AndersonTorres ];
    platforms = lib.platforms.all;
  };
}

The error log:

emacs-sunrise-commander-unstable> Running phase: installPhase
emacs-sunrise-commander-unstable> Error: file-error ("Read error" "Is a directory" "/nix/store/da7c3cm3m6m0in0c6d28m25r2qc585b5-source")
emacs-sunrise-commander-unstable>   mapbacktrace(#f(compiled-function (evald func args flags) #<bytecode -0xcc6be0a6510e41>))
emacs-sunrise-commander-unstable>   debug-early-backtrace()
emacs-sunrise-commander-unstable>   debug-early(error (file-error "Read error" "Is a directory" "/nix/store/da7c3cm3m6m0in0c6d28m25r2qc585b5-source"))
emacs-sunrise-commander-unstable>   insert-file-contents("/nix/store/da7c3cm3m6m0in0c6d28m25r2qc585b5-source")
emacs-sunrise-commander-unstable>   (if is-tar (insert-file-contents-literally file) (insert-file-contents file))
emacs-sunrise-commander-unstable>   (progn (if is-tar (insert-file-contents-literally file) (insert-file-contents file)) (if is-tar (progn (tar-mode))) (elpa2nix-install-from-buffer))
emacs-sunrise-commander-unstable>   (unwind-protect (progn (if is-tar (insert-file-contents-literally file) (insert-file-contents file)) (if is-tar (progn (tar-mode))) (elpa2nix-install-from-buffer)) (and (buffer-name temp-buffer) (kill-buffer temp-buffer)))
emacs-sunrise-commander-unstable>   (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (if is-tar (insert-file-contents-literally file) (insert-file-contents file)) (if is-tar (progn (tar-mode))) (elpa2nix-install-from-buffer)) (and (buffer-name temp-buffer) (kill-buffer temp-buffer))))
emacs-sunrise-commander-unstable>   (let ((temp-buffer (generate-new-buffer " *temp*" t))) (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (if is-tar (insert-file-contents-literally file) (insert-file-contents file)) (if is-tar (progn (tar-mode))) (elpa2nix-install-from-buffer)) (and (buffer-name temp-buffer) (kill-buffer temp-buffer)))))
emacs-sunrise-commander-unstable>   (let ((is-tar (string-match "\\.tar\\'" file))) (let ((temp-buffer (generate-new-buffer " *temp*" t))) (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (if is-tar (insert-file-contents-literally file) (insert-file-contents file)) (if is-tar (progn (tar-mode))) (elpa2nix-install-from-buffer)) (and (buffer-name temp-buffer) (kill-buffer temp-buffer))))))
emacs-sunrise-commander-unstable>   elpa2nix-install-file("/nix/store/da7c3cm3m6m0in0c6d28m25r2qc585b5-source")
emacs-sunrise-commander-unstable>   (progn (setq package-user-dir elpa) (elpa2nix-install-file archive))
emacs-sunrise-commander-unstable>   (let ((archive x0) (elpa x2)) (progn (setq package-user-dir elpa) (elpa2nix-install-file archive)))
emacs-sunrise-commander-unstable>   (if (null x3) (let ((archive x0) (elpa x2)) (progn (setq package-user-dir elpa) (elpa2nix-install-file archive))))
emacs-sunrise-commander-unstable>   (let* ((x2 (car-safe x1)) (x3 (cdr-safe x1))) (if (null x3) (let ((archive x0) (elpa x2)) (progn (setq package-user-dir elpa) (elpa2nix-install-file archive)))))
emacs-sunrise-commander-unstable>   (if (consp x1) (let* ((x2 (car-safe x1)) (x3 (cdr-safe x1))) (if (null x3) (let ((archive x0) (elpa x2)) (progn (setq package-user-dir elpa) (elpa2nix-install-file archive))))))
emacs-sunrise-commander-unstable>   (let* ((x0 (car-safe command-line-args-left)) (x1 (cdr-safe command-line-args-left))) (if (consp x1) (let* ((x2 (car-safe x1)) (x3 (cdr-safe x1))) (if (null x3) (let ((archive x0) (elpa x2)) (progn (setq package-user-dir elpa) (elpa2nix-install-file archive)))))))
emacs-sunrise-commander-unstable>   (if (consp command-line-args-left) (let* ((x0 (car-safe command-line-args-left)) (x1 (cdr-safe command-line-args-left))) (if (consp x1) (let* ((x2 (car-safe x1)) (x3 (cdr-safe x1))) (if (null x3) (let ((archive x0) (elpa x2)) (progn (setq package-user-dir elpa) (elpa2nix-install-file archive))))))))
emacs-sunrise-commander-unstable>   elpa2nix-install-package()
emacs-sunrise-commander-unstable>   command-line-1(("-l" "/nix/store/4jj63z4v1xp13rh2md053dccq920hd45-elpa2nix.el" "-f" "elpa2nix-install-package" "/nix/store/da7c3cm3m6m0in0c6d28m25r2qc585b5-source" "/nix/store/mv5i7kvw8m1siga6g3n6avbh0qzk71nz-emacs-sunrise-commander-unstable-2021-09-27/share/emacs/site-lisp/elpa"))
emacs-sunrise-commander-unstable>   command-line()
emacs-sunrise-commander-unstable>   normal-top-level()
emacs-sunrise-commander-unstable> Read error: Is a directory, /nix/store/da7c3cm3m6m0in0c6d28m25r2qc585b5-source
error: builder for '/nix/store/qzdqqh8dfy63jdnhm4n06x0pv97ap53i-emacs-sunrise-commander-unstable-2021-09-27.drv' failed with exit code 255;
jian-lin commented 11 months ago

elpaBuild expects src to be an elisp file or a tarball meeting some requirements. The requirement of src of melpaBuild is less strict than the one of elpaBuild.

Have you tried melpaBuild?

AndersonTorres commented 11 months ago

I tried it a long time ago. But let's do it again:

{ lib
, emacs
, fetchFromGitHub
, melpaBuild
, writeText
}:

melpaBuild {
  pname = "sunrise-commander";
  ename = "sunrise-commander";
  version = "unstable-2021-09-27";

  src = fetchFromGitHub {
    owner = "sunrise-commander";
    repo = "sunrise-commander";
    rev = "16e6df7e86c7a383fb4400fae94af32baf9cb24e";
    hash = "sha256-D36qiRi5OTZrBtJ/bD/javAWizZ8NLlC/YP4rdLCSsw=";
  };

  packageRequires = [
    emacs
  ];

  recipe = writeText "recipe" ''
    (sunrise-commander :repo "sunrise-commander/sunrise-commander" :fetcher github :files ("*.el"))
  '';

  fileSpecs = [ "*.el" ];

  meta = {
    homepage = "https://github.com/sunrise-commander/sunrise-commander/";
    description = "Orthodox (two-pane) file manager for Emacs";
    license = lib.licenses.gpl3Plus;
    maintainers = [ lib.maintainers.AndersonTorres ];
    platforms = lib.platforms.all;
  };
}

Log:

emacs-sunrise-commander-unstable> Running phase: installPhase
emacs-sunrise-commander-unstable> Error: file-missing ("Opening input file" "No such file or directory" "/build/packages/sunrise-commander-unstable-2021-09-27.tar")
emacs-sunrise-commander-unstable>   mapbacktrace(#f(compiled-function (evald func args flags) #<bytecode -0xcc6be0a6510e41>))
emacs-sunrise-commander-unstable>   debug-early-backtrace()
emacs-sunrise-commander-unstable>   debug-early(error (file-missing "Opening input file" "No such file or directory" "/build/packages/sunrise-commander-unstable-2021-09-27.tar"))
emacs-sunrise-commander-unstable>   insert-file-contents-literally("/build/packages/sunrise-commander-unstable-2021-09-27.tar")
emacs-sunrise-commander-unstable>   (if is-tar (insert-file-contents-literally file) (insert-file-contents file))
emacs-sunrise-commander-unstable>   (progn (if is-tar (insert-file-contents-literally file) (insert-file-contents file)) (if is-tar (progn (tar-mode))) (elpa2nix-install-from-buffer))
emacs-sunrise-commander-unstable>   (unwind-protect (progn (if is-tar (insert-file-contents-literally file) (insert-file-contents file)) (if is-tar (progn (tar-mode))) (elpa2nix-install-from-buffer)) (and (buffer-name temp-buffer) (kill-buffer temp-buffer)))
emacs-sunrise-commander-unstable>   (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (if is-tar (insert-file-contents-literally file) (insert-file-contents file)) (if is-tar (progn (tar-mode))) (elpa2nix-install-from-buffer)) (and (buffer-name temp-buffer) (kill-buffer temp-buffer))))
emacs-sunrise-commander-unstable>   (let ((temp-buffer (generate-new-buffer " *temp*" t))) (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (if is-tar (insert-file-contents-literally file) (insert-file-contents file)) (if is-tar (progn (tar-mode))) (elpa2nix-install-from-buffer)) (and (buffer-name temp-buffer) (kill-buffer temp-buffer)))))
emacs-sunrise-commander-unstable>   (let ((is-tar (string-match "\\.tar\\'" file))) (let ((temp-buffer (generate-new-buffer " *temp*" t))) (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (if is-tar (insert-file-contents-literally file) (insert-file-contents file)) (if is-tar (progn (tar-mode))) (elpa2nix-install-from-buffer)) (and (buffer-name temp-buffer) (kill-buffer temp-buffer))))))
emacs-sunrise-commander-unstable>   elpa2nix-install-file("/build/packages/sunrise-commander-unstable-2021-09-27.tar")
emacs-sunrise-commander-unstable>   (progn (setq package-user-dir elpa) (elpa2nix-install-file archive))
emacs-sunrise-commander-unstable>   (let ((archive x0) (elpa x2)) (progn (setq package-user-dir elpa) (elpa2nix-install-file archive)))
emacs-sunrise-commander-unstable>   (if (null x3) (let ((archive x0) (elpa x2)) (progn (setq package-user-dir elpa) (elpa2nix-install-file archive))))
emacs-sunrise-commander-unstable>   (let* ((x2 (car-safe x1)) (x3 (cdr-safe x1))) (if (null x3) (let ((archive x0) (elpa x2)) (progn (setq package-user-dir elpa) (elpa2nix-install-file archive)))))
emacs-sunrise-commander-unstable>   (if (consp x1) (let* ((x2 (car-safe x1)) (x3 (cdr-safe x1))) (if (null x3) (let ((archive x0) (elpa x2)) (progn (setq package-user-dir elpa) (elpa2nix-install-file archive))))))
emacs-sunrise-commander-unstable>   (let* ((x0 (car-safe command-line-args-left)) (x1 (cdr-safe command-line-args-left))) (if (consp x1) (let* ((x2 (car-safe x1)) (x3 (cdr-safe x1))) (if (null x3) (let ((archive x0) (elpa x2)) (progn (setq package-user-dir elpa) (elpa2nix-install-file archive)))))))
emacs-sunrise-commander-unstable>   (if (consp command-line-args-left) (let* ((x0 (car-safe command-line-args-left)) (x1 (cdr-safe command-line-args-left))) (if (consp x1) (let* ((x2 (car-safe x1)) (x3 (cdr-safe x1))) (if (null x3) (let ((archive x0) (elpa x2)) (progn (setq package-user-dir elpa) (elpa2nix-install-file archive))))))))
emacs-sunrise-commander-unstable>   elpa2nix-install-package()
emacs-sunrise-commander-unstable>   command-line-1(("-l" "/nix/store/4jj63z4v1xp13rh2md053dccq920hd45-elpa2nix.el" "-f" "elpa2nix-install-package" "/build/packages/sunrise-commander-unstable-2021-09-27.tar" "/nix/store/6gf924kvnbpjgf7mrwmm93ih5pcjpkrr-emacs-sunrise-commander-unstable-2021-09-27/share/emacs/site-lisp/elpa"))
emacs-sunrise-commander-unstable>   command-line()
emacs-sunrise-commander-unstable>   normal-top-level()
emacs-sunrise-commander-unstable> Opening input file: No such file or directory, /build/packages/sunrise-commander-unstable-2021-09-27.tar
jian-lin commented 11 months ago

Here is a working example:

{ lib
, emacs
, fetchFromGitHub
, melpaBuild
, writeText
}:

melpaBuild {
  pname = "sunrise-commander";
  ename = "sunrise";
  version = "20210924.1620";

  src = fetchFromGitHub {
    owner = "sunrise-commander";
    repo = "sunrise-commander";
    rev = "16e6df7e86c7a383fb4400fae94af32baf9cb24e";
    hash = "sha256-D36qiRi5OTZrBtJ/bD/javAWizZ8NLlC/YP4rdLCSsw=";
  };

  # not sure whether it is still needed when a newer[1] package-build is used
  # [1]: https://github.com/NixOS/nixpkgs/pull/276943 is in the staging branch for now
  commit = "foo";

  # probably redundant
  # packageRequires = [
  #   emacs
  # ];

  recipe = writeText "recipe" ''
    (sunrise :repo "sunrise-commander/sunrise-commander" :fetcher github)
  '';

  meta = {
    homepage = "https://github.com/sunrise-commander/sunrise-commander/";
    description = "Orthodox (two-pane) file manager for Emacs";
    license = lib.licenses.gpl3Plus;
    maintainers = [ lib.maintainers.AndersonTorres ];
    platforms = lib.platforms.all;
  };
}
AndersonTorres commented 11 months ago

Nice. Now we need to wait the Staging Truckload.


  1. Where did the attribute version = "20210924.1620"; come from? I have grepped the installed files and it comes from nowhere except an autogenerated sunrise-pkg.el...
  2. What does fileSpecs do? It is not documented anywhere...
jian-lin commented 11 months ago

https://github.com/NixOS/nixpkgs/issues/190746 is related

jian-lin commented 11 months ago

Where did the attribute version = "20210924.1620"; come from?

This is the version format MELPA unstable uses.

(let ((time 1703448134))
  (concat (format-time-string "%Y%m%d." time t)
          (format "%d" (string-to-number
                        (format-time-string "%H%M" time t)))))

time is produced by a git command git log -n1 --first-parent --pretty='format:%H %cd' --date=unix.

I think fileSpecs has no effect since https://github.com/NixOS/nixpkgs/commit/d3cea4860862754d74f60a4332c24b646f1446c9#diff-2a37290b0927d8abb03148a01a10a84dd3a18be9be80d27c7aac6c02d8ea6841L14

AndersonTorres commented 11 months ago

Some obvious findings:

hraban commented 9 months ago

Thanks for your work on this, y'all! 🙌

jian-lin commented 6 months ago

I created https://github.com/NixOS/nixpkgs/pull/316107 to improve the UX of melpaBuild.

jian-lin commented 6 months ago

With https://github.com/NixOS/nixpkgs/pull/316726, the UX of melpaBuild should be on a par with elpaBuild and trivialBuild.

Given that it has fewer issues than trivialBuild and that its requirement for src is looser than elpaBuild, it should be recommended as the default builder for manual packages.

AndersonTorres commented 4 months ago

Now that it reached Master, we can restart!

hraban commented 4 months ago

How do you use melpaBuild to create very simple in-line packages?

          (e.trivialBuild {
            pname = "foo";
            version = "2024-07-15";
            src = builtins.toFile "foo.el" ''(defun foo-three () 3) (provide 'foo)'';
          })

I use this to create light-weight emacs plugins from my Nix config, it's a neat way to integrate the broader system config into Emacs.

AndersonTorres commented 4 months ago

@hraban you can look at https://github.com/NixOS/nixpkgs/pull/325168 to take some inspiration.

The problem is that maybe melpaBuild is a bit stricter, since it transfers control to Emacs build system.

jian-lin commented 4 months ago

The minimal melpaBuild equivalent is

  e.melpaBuild {
    pname = "foo";
    version = "0-unstable-2024-07-15";
    # version = "20240715.0"; # if your Nixpkgs is old
    src = builtins.toFile "foo.el" ''
      ;;; foo.el ---
      (defun foo-three () 3) (provide 'foo)
    '';
  };

The first line (file header) ;;; foo.el --- is needed by melpaBuild to generate foo-pkg.el.

If trivialBuild works well for your use case, then there is no need to switch to melpaBuild. melpaBuild is still a good fit for "normal" packages.

AndersonTorres commented 4 months ago
Updated at: 2024-07-19

PRs dealing with some trivialBuild packages:

EmacsWiki repacks: https://github.com/NixOS/nixpkgs/pull/328074

Packages that need some extra modifications in order to be built via the current melpaBuild:

PRs dealiong with removal

hraban commented 4 months ago

The minimal melpaBuild equivalent is

  e.melpaBuild {
    pname = "foo";
    version = "0-unstable-2024-07-15";
    # version = "20240715.0"; # if your Nixpkgs is old
    src = builtins.toFile "foo.el" ''
      ;;; foo.el ---
      (defun foo-three () 3) (provide 'foo)
    '';
  };

The first line (file header) ;;; foo.el --- is needed by melpaBuild to generate foo-pkg.el.

Thanks! Good catch: that line was necessary for melpaBuild but not for trivialBuild

If trivialBuild works well for your use case, then there is no need to switch to melpaBuild. melpaBuild is still a good fit for "normal" packages.

That'd be good enough for my use case, I was going off the impression from this issue title that trivialBuild is on the way to becoming obsolete but if it's still available that's nice.

jian-lin commented 4 months ago

EmacsWiki repacks (they should be dealt with separately):

@AndersonTorres What do you mean by "they should be dealt with separately"? Why and how?

AndersonTorres commented 4 months ago

It is better explained with an example:

Let's pick control-lock.

Webpage: https://www.emacswiki.org/emacs/ControlLock Download page: https://www.emacswiki.org/emacs/download/control-lock.el

There is virtually no version control.

However, there is a backup at GitHub:

https://github.com/emacsmirror/emacswiki.org/blob/master/control-lock.el

My intention is to create a "package" for this EmacsWiki mirror and do something like files = ''("control-lock.el")''.

AndersonTorres commented 4 months ago

Another thing to be seen is the auto-update scripts.

jian-lin commented 4 months ago

Another thing to be seen is the auto-update scripts.

Since trivialBuild works well in that use case (autolaoding is not used by update-melpa.el) and users is unlikely to look at that file, it's fine to not change it to melpaBuild.


I am working on the last piece of this issue, i.e., writing doc for these builders.

UPDATE: I block my doc work on https://github.com/NixOS/nixpkgs/pull/330589. This is just a personal choice. If you want to write doc for the current builders, please feel free to do so.

AndersonTorres commented 4 months ago

I wasn't clear. I was talking about passthru.updateScript.

jian-lin commented 3 months ago

@AndersonTorres Could you switch mkDerivation to melpaBuild for cask?

AndersonTorres commented 3 months ago

Yes I can. However I will need to study cask a little bit, since Cask is a commandline app.

AndersonTorres commented 3 months ago

For now the only "non-conforming" packages are xapian-lite and cedille. However Cedille is broken:

Broken due to Agda update. See

https://github.com/NixOS/nixpkgs/pull/129606#issuecomment-881107449.