--- Begin Message ---
|
Subject: |
[PATCH] build: emacs-build-system: Make the install phase more helpful. |
|
Date: |
Tue, 22 Aug 2017 18:13:03 +0100 |
Modify the install phase to detect when nothing has been installed, and error
if this happens. This is preferable to continuing, and allowing the next phase
to fail.
Also, when nothing can be found to be installed, print out each file that was
considered, along with the regular expressions that were used to include and
exclude it.
* gnu/build/emacs-build-system.scm (install-file?): Add additional error
checking and logging.
---
guix/build/emacs-build-system.scm | 45 ++++++++++++++++++++++++++++-----------
1 file changed, 32 insertions(+), 13 deletions(-)
diff --git a/guix/build/emacs-build-system.scm
b/guix/build/emacs-build-system.scm
index bda699ddf..d67d7b49c 100644
--- a/guix/build/emacs-build-system.scm
+++ b/guix/build/emacs-build-system.scm
@@ -110,22 +110,41 @@ store in '.el' files."
(define source (getcwd))
- (define (install-file? file stat)
- (let ((stripped-file (string-trim (string-drop file (string-length
source)) #\/)))
- (and (any (cut string-match <> stripped-file) include)
- (not (any (cut string-match <> stripped-file) exclude)))))
+ (define* (install-file? file stat #:key verbose?)
+ (let* ((stripped-file (string-trim
+ (string-drop file (string-length source)) #\/)))
+ (define (match-stripped-file action regex)
+ (let ((result (string-match regex stripped-file)))
+ (if (and result verbose?)
+ (format #t "info: ~A ~A as it matches \"~A\"\n"
+ stripped-file action regex))
+ result))
+
+ (if verbose?
+ (format #t "info: considering installing ~A\n" stripped-file))
+
+ (and (any (cut match-stripped-file "included" <>) include)
+ (not (any (cut match-stripped-file "excluded" <>) exclude)))))
(let* ((out (assoc-ref outputs "out"))
(elpa-name-ver (store-directory->elpa-name-version out))
- (target-directory (string-append out %install-suffix "/"
elpa-name-ver)))
- (for-each
- (lambda (file)
- (let* ((stripped-file (string-drop file (string-length source)))
- (target-file (string-append target-directory stripped-file)))
- (format #t "`~a' -> `~a'~%" file target-file)
- (install-file file (dirname target-file))))
- (find-files source install-file?)))
- #t)
+ (target-directory (string-append out %install-suffix "/"
elpa-name-ver))
+ (files-to-install (find-files source install-file?)))
+ (if (null? files-to-install)
+ (begin
+ (format #t "error: No files found to install.\n")
+ (find-files source (lambda (file stat)
+ (install-file? file stat #:verbose? #t)))
+ #f)
+ (begin
+ (for-each
+ (lambda (file)
+ (let* ((stripped-file (string-drop file (string-length source)))
+ (target-file (string-append target-directory
stripped-file)))
+ (format #t "`~a' -> `~a'~%" file target-file)
+ (install-file file (dirname target-file))))
+ files-to-install)
+ #t))))
(define* (move-doc #:key outputs #:allow-other-keys)
"Move info files from the ELPA package directory to the info directory."
--
2.13.1
--- End Message ---
--- Begin Message ---
|
Subject: |
Re: [bug#28185] [PATCH] build: emacs-build-system: Make the install phase more helpful. |
|
Date: |
Fri, 1 Sep 2017 22:08:55 +0100 |
On Fri, 01 Sep 2017 10:32:18 +0530
Arun Isaac <address@hidden> wrote:
> >> > + (if (null? files-to-install)
> >> > + (begin
> >> > + (format #t "error: No files found to install.\n")
> >> > + (find-files source (lambda (file stat)
> >> > + (install-file? file stat
> >> > #:verbose? #t)))
> >>
> >> I understand that the verbose output also shows the regexp due to
> >> which the file was excluded, and that has debugging value. But,
> >> perhaps, it'll be better to just print that here instead of a call
> >> back to `install-file?'.
> >>
> >> WDYT?
> >
> > I put this in install-file? as I didn't want to duplicate the
> > behaviour inside the function, firstly to avoid duplication, but
> > also to avoid the possiblily that if they were duplicated, they
> > would diverge in behaviour. I'm happy to experiment with splitting
> > the "verbose" output out of install-file though?
>
> Yes, I meant that you should split the verbose output out of
> `install-file?'. But, it's not a big deal. I'm only nitpicking. Do go
> ahead with your original code if you think it's alright.
>
> >> > + #f)
> >> > + (begin
> >> > + (for-each
> >> > + (lambda (file)
> >> > + (let* ((stripped-file (string-drop file
> >> > (string-length source)))
> >> > + (target-file (string-append target-directory
> >> > stripped-file)))
> >> > + (format #t "`~a' -> `~a'~%" file target-file)
> >> > + (install-file file (dirname target-file))))
> >> > + files-to-install)
> >> > + #t))))
> >>
> >> Could you please rewrite this section using `cond' instead of `if'?
> >> That way, we can get rid of the `begin'. Also, it might be better
> >> if we used "positive logic" -- meaning, we first check for
> >> truthiness of `files-to-install' instead of checking for (null?
> >> files-to-install). Then, the else statement would take care of the
> >> empty files-to-install case, and we would never have to call
> >> `null?' explicitly.
> >
> > I tried this, but it seems that cond will treat '() as if it
> > evaluates to true:
> >
> > scheme@(guile-user)> (cond ('() #t) (else #f))
> > $1 = #t
> >
> > So this might need to still use null?, or even (not (null?
> > files-to-install))?
>
> Ah, yes. Sorry, I'm confusing scheme's behaviour with that of other
> lisps. We still need `null?'.
>
> The patch LGTM.
Great, I've pushed this now :)
pgphHUuWtNwEc.pgp
Description: OpenPGP digital signature
--- End Message ---