diff mbox series

[bug#46162] staging gnu: Add more tools to rust outputs.

Message ID 87ft1xwbqh.fsf@asu.edu
State New
Headers show
Series [bug#46162] staging gnu: Add more tools to rust outputs. | expand

Checks

Context Check Description
cbaines/submitting builds success
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

John Soo Feb. 15, 2021, 4:13 p.m. UTC
Rebased on staging.

There is one more question that I have about search paths.  A lot of rust
tools use the $RUST_SRC_PATH to find the source of rust.  I add the
source output as "src" but I am not sure how to specify a search path to
the top directory of a separate output.  Any pointers?

Thanks!

John

Comments

John Soo Feb. 15, 2021, 6:09 p.m. UTC | #1
Hi Jakub,

Thanks again for your work on rust.

I cc'd you for feedback on adding the "extended" tools to rust's
outputs.

- John
Jakub Kądziołka Feb. 15, 2021, 11:41 p.m. UTC | #2
On Mon Feb 15, 2021 at 7:09 PM CET, John Soo wrote:
> Hi Jakub,
>
> Thanks again for your work on rust.
>
> I cc'd you for feedback on adding the "extended" tools to rust's
> outputs.

I don't think tools beyond rustc and cargo should be included in the
main rust package, as this causes them to be built in each step of the
bootstrap. I believe a better approach would be to define separate
packages for them.

We would have something like

 ;; TODO(staging): Bump this variable to the latest packaged rust.
 (define-public rust rust-1.45)

+(define-public rust-for-tools rust-1.50)

I'm not sure if rustbuild can be convinced to not build the compiler
itself when the version used for the build is the same as the sources'.
If so, defining packages for each tool shouldn't need any guix-side
tricks.

Otherwise, I would define a single rust-tools package with
(outputs '("rustfmt" "clippy" ...)). Perhaps it would help with UX if
rust-tools itself was hidden, and instead the tools would be exposed
with simple packages that expose each tool separately, with a symlink or
similar.

I'll see if I can find some time to try this out this week.

Regards,
Jakub Kądziołka
John Soo Feb. 16, 2021, 4:40 p.m. UTC | #3
Hi Jakub,

Jakub Kądziołka <kuba@kadziolka.net> writes:

> I don't think tools beyond rustc and cargo should be included in the
> main rust package, as this causes them to be built in each step of the
> bootstrap. I believe a better approach would be to define separate
> packages for them.

Yeah that would make sense. Have we explored any of the incremental or
--keep-stage options to speedup the bootstrap?

> We would have something like
>
>  ;; TODO(staging): Bump this variable to the latest packaged rust.
>  (define-public rust rust-1.45)
>
> +(define-public rust-for-tools rust-1.50)
>
> I'm not sure if rustbuild can be convinced to not build the compiler
> itself when the version used for the build is the same as the sources'.
> If so, defining packages for each tool shouldn't need any guix-side
> tricks.

Even if it did build the compiler for each tool it may not be a problem
if only the last 3 versions had the tools available (for instance).

> Otherwise, I would define a single rust-tools package with
> (outputs '("rustfmt" "clippy" ...)). Perhaps it would help with UX if
> rust-tools itself was hidden, and instead the tools would be exposed
> with simple packages that expose each tool separately, with a symlink or
> similar.

I could see this working nicely.  I think this is just more evidence for
language-environment related documentation.

> I'll see if I can find some time to try this out this week.

Thanks! That would be helpful. It is really painful to have all these
unmerged patches to rust.

Kindly,

John
Jakub Kądziołka Feb. 16, 2021, 4:41 p.m. UTC | #4
On Tue Feb 16, 2021 at 5:40 PM CET, John Soo wrote:
> Hi Jakub,
>
> Jakub Kądziołka <kuba@kadziolka.net> writes:
>
> > I don't think tools beyond rustc and cargo should be included in the
> > main rust package, as this causes them to be built in each step of the
> > bootstrap. I believe a better approach would be to define separate
> > packages for them.
>
> Yeah that would make sense. Have we explored any of the incremental or
> --keep-stage options to speedup the bootstrap?

apteryx mentioned some experiments with those on #mrustc.

Regards,
Jakub Kądziołka
Maxim Cournoyer Feb. 18, 2021, 6:01 p.m. UTC | #5
Hi,

Jakub Kądziołka <kuba@kadziolka.net> writes:

> On Tue Feb 16, 2021 at 5:40 PM CET, John Soo wrote:
>> Hi Jakub,
>>
>> Jakub Kądziołka <kuba@kadziolka.net> writes:
>>
>> > I don't think tools beyond rustc and cargo should be included in the
>> > main rust package, as this causes them to be built in each step of the
>> > bootstrap. I believe a better approach would be to define separate
>> > packages for them.
>>
>> Yeah that would make sense. Have we explored any of the incremental or
>> --keep-stage options to speedup the bootstrap?
>
> apteryx mentioned some experiments with those on #mrustc.
>
> Regards,
> Jakub Kądziołka

Those weren't fruitful, but I didn't push it; I think it caused cargo to
fail compiling itself, or its test suite.

Maxim
diff mbox series

Patch

From cd849893e7f9b6ad8c380b3ebc74ea92530f8c48 Mon Sep 17 00:00:00 2001
From: John Soo <jsoo1@asu.edu>
Date: Wed, 25 Nov 2020 06:25:43 -0800
Subject: [PATCH] gnu: Add more tools to rust outputs.

The goal is to provide standard rust tools as outputs of rustc.  The tools we
were missing were rls, clippy, src, and rust-analyzer. as a separate output of
rust.

* gnu/packages/rust.scm (rust-1.46): [outputs] add rls, clippy, src, and
rust-analyzer, [arguments] alter phases to build, test, and install each new
output, [arguments] patch RUNPATHS of outputs that require it, [arguments]
delete all uninstall scripts from outputs, [arguments] delete all the install
logs and manifests.
---
 gnu/packages/rust.scm | 145 +++++++++++++++++++++++++++++++-----------
 1 file changed, 109 insertions(+), 36 deletions(-)

diff --git a/gnu/packages/rust.scm b/gnu/packages/rust.scm
index f72f310169..8a09e1077e 100644
--- a/gnu/packages/rust.scm
+++ b/gnu/packages/rust.scm
@@ -11,6 +11,7 @@ 
 ;;; Copyright © 2020, 2021 Jakub Kądziołka <kuba@kadziolka.net>
 ;;; Copyright © 2020 Pierre Langlois <pierre.langlois@gmx.com>
 ;;; Copyright © 2020 Matthew Kraai <kraai@ftbfs.org>
+;;; Copyright © 2021 John Soo <jsoo1@asu.edu>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -41,6 +42,7 @@ 
   #:use-module (gnu packages jemalloc)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages llvm)
+  #:use-module (gnu packages node)
   #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages python)
   #:use-module (gnu packages ssh)
@@ -1322,19 +1324,41 @@  move around."
            "0a17jby2pd050s24cy4dfc0gzvgcl585v3vvyfilniyvjrqknsid")))
     (package
       (inherit base-rust)
-      (outputs (cons "rustfmt" (package-outputs base-rust)))
+      (outputs (append '("rustfmt" "rls" "src" "clippy" "rust-analyzer")
+                       (package-outputs base-rust)))
+      (inputs
+       `(("gcc-lib" ,gcc "lib")
+         ,@(package-inputs base-rust)))
+      (native-inputs
+       `(("node" ,node)
+         ("patchelf" ,patchelf)
+         ,@(package-native-inputs base-rust)))
       (arguments
        (substitute-keyword-arguments (package-arguments base-rust)
          ((#:phases phases)
           `(modify-phases ,phases
+             (replace 'patch-cargo-checksums
+               ;; Generate checksums after patching generated files (in
+               ;; particular, vendor/jemalloc/rep/Makefile).
+               (lambda* _
+                 (use-modules (guix build cargo-utils))
+                 (substitute* '("Cargo.lock"
+                                "src/tools/rust-analyzer/Cargo.lock")
+                   (("(checksum = )\".*\"" all name)
+                    (string-append name "\"" ,%cargo-reference-hash "\"")))
+                 (generate-all-checksums "vendor")
+                 #t))
              (replace 'build
                (lambda* _
                  (invoke "./x.py" "build")
                  (invoke "./x.py" "build" "src/tools/cargo")
-                 (invoke "./x.py" "build" "src/tools/rustfmt")))
+                 (invoke "./x.py" "build" "src/tools/rustfmt")
+                 (invoke "./x.py" "build" "src/tools/clippy")
+                 (invoke "./x.py" "build" "src/tools/rls")
+                 (invoke "./x.py" "build"
+                         "src/tools/rust-analyzer/crates/rust-analyzer")))
              (replace 'check
                (lambda* _
-                 ;; Test rustfmt.
                  (let ((parallel-job-spec
                         (string-append "-j" (number->string
                                              (min 4
@@ -1343,44 +1367,84 @@  move around."
                    (invoke "./x.py" parallel-job-spec "test"
                            "src/tools/cargo")
                    (invoke "./x.py" parallel-job-spec "test"
-                           "src/tools/rustfmt"))))
+                           "src/tools/rustfmt")
+                   ;; Clippy tests do not work. See
+                   ;; https://github.com/rust-lang/rust/issues/78717
+                   ;; Even with --stage 1, they fail to compile
+                   ;; (invoke "./x.py" parallel-job-spec "test" "--stage" "1"
+                   ;;         "src/tools/clippy")
+                   (substitute* "src/tools/rls/tests/client.rs"
+                     (("fn client_dependency_typo_and_fix" all)
+                      (string-append "#[ignore]\n" all)))
+                   (invoke "./x.py" parallel-job-spec "test"
+                           "src/tools/rls"))))
              (replace 'install
                (lambda* (#:key outputs #:allow-other-keys)
                  (invoke "./x.py" "install")
+                 (for-each delete-file-recursively
+                           (find-files (assoc-ref outputs "out")
+                                       "^uninstall\\.sh$"))
                  (substitute* "config.toml"
                    ;; replace prefix to specific output
-                   (("prefix = \"[^\"]*\"")
-                    (string-append "prefix = \"" (assoc-ref outputs "cargo") "\"")))
-                 (invoke "./x.py" "install" "cargo")
-                 (substitute* "config.toml"
-                   ;; replace prefix to specific output
-                   (("prefix = \"[^\"]*\"")
-                    (string-append "prefix = \"" (assoc-ref outputs "rustfmt") "\"")))
-                 (invoke "./x.py" "install" "rustfmt")))
+                   (("\\[build\\]" all)
+                    (string-append all "
+extended = true
+tools =
+")))
+                 (define (install-component component)
+                   (substitute* "config.toml"
+                     ;; replace prefix to specific output
+                     (("(tools =).*" all tools)
+                      (string-append tools " [\"" component "\"]\n"))
+                     (("prefix = \"[^\"]*\"")
+                      (string-append
+                       "prefix = \"" (assoc-ref outputs component) "\"")))
+                   (mkdir-p (assoc-ref outputs component))
+                   (invoke "./x.py" "install" component)
+                   (for-each delete-file-recursively
+                             (find-files (assoc-ref outputs component)
+                                         "uninstall\\.sh")))
+                 (for-each install-component
+                           '("cargo"
+                             "rustfmt"
+                             "clippy"
+                             "rls"
+                             "src"
+                             "rust-analyzer"))
+                 #t))
+             (add-after 'install 'patch-tools-runpaths
+               (lambda* (#:key outputs inputs #:allow-other-keys)
+                 (use-modules (ice-9 popen)
+                              (ice-9 textual-ports))
+                 (define (patch-path path)
+                   (let* ((read-rpath
+                           (string-append
+                            "patchelf --print-rpath " path))
+                          (pipe (open-input-pipe read-rpath))
+                          (current-rpath (get-string-all pipe))
+                          (out (assoc-ref outputs "out"))
+                          (libc (assoc-ref inputs "libc"))
+                          (gcc-lib (assoc-ref inputs "gcc-lib")))
+                     (close-pipe pipe)
+                     (invoke "patchelf" "--set-rpath"
+                             (string-append current-rpath
+                                            ":" out "/lib"
+                                            ":" libc "/lib"
+                                            ":" gcc-lib "/lib")
+                             path)))
+                 (define (patch-component component)
+                   (for-each patch-path
+                             (find-files (assoc-ref outputs component)
+                                         (lambda (p s) (executable-file? p)))))
+                 (for-each patch-component '("clippy" "rls"))))
              (replace 'delete-install-logs
                (lambda* (#:key outputs #:allow-other-keys)
-                 (define (delete-manifest-file out-path file)
-                   (delete-file (string-append out-path "/lib/rustlib/" file)))
-
-                 (let ((out (assoc-ref outputs "out"))
-                       (cargo-out (assoc-ref outputs "cargo"))
-                       (rustfmt-out (assoc-ref outputs "rustfmt")))
-                   (for-each
-                     (lambda (file) (delete-manifest-file out file))
-                     '("install.log"
-                       "manifest-rust-docs"
-                       ,(string-append "manifest-rust-std-"
-                                       (nix-system->gnu-triplet-for-rust))
-                       "manifest-rustc"))
-                   (for-each
-                     (lambda (file) (delete-manifest-file cargo-out file))
-                     '("install.log"
-                       "manifest-cargo"))
-                   (for-each
-                     (lambda (file) (delete-manifest-file rustfmt-out file))
-                     '("install.log"
-                       "manifest-rustfmt-preview"))
-                   #t))))))))))
+                 (define log-manifest-re
+                   "^install\\.log$|^manifest-([a-z]|[0-9]|_|-)+(-preview)?$")
+                 (define (delete-install-log output)
+                   (for-each delete-file-recursively
+                             (find-files output log-manifest-re)))
+                 (for-each delete-install-log (map cdr outputs)))))))))))
 
 (define-public rust-1.47
   (let ((base-rust
@@ -1413,7 +1477,8 @@  move around."
                ;; particular, vendor/jemalloc/rep/Makefile).
                (lambda* _
                  (use-modules (guix build cargo-utils))
-                 (substitute* "Cargo.lock"
+                 (substitute* '("Cargo.lock"
+                                "src/tools/rust-analyzer/Cargo.lock")
                    (("(checksum = )\".*\"" all name)
                     (string-append name "\"" ,%cargo-reference-hash "\"")))
                  (generate-all-checksums "vendor")
@@ -1445,7 +1510,15 @@  move around."
                      (("fn test_process_mask") "#[allow(unused_attributes)]
     #[ignore]
     fn test_process_mask"))
-                   #t))))))))))
+                   #t)))
+             ;; FIXME: This fixes the "src" output which is not critical.  We
+             ;; should probably copy the source of the de-vendored libunwind
+             ;; for completeness' sake.
+             (add-before 'install 'remove-vendored-llvm-reference-in-src
+               (lambda _
+                 (substitute* "src/bootstrap/dist.rs"
+                   ((", \"src/llvm-project/libunwind\"") ""))
+                 #t)))))))))
 
 (define-public rust-1.49
   (rust-bootstrapped-package rust-1.48 "1.49.0"
-- 
2.30.0