[bug#28185] [PATCH] build: emacs-build-system: Make the install phase more helpful.

[Arun Isaac]

Ok. Ludo has spoken. :-) Let us proceed with the patch review.

> -  (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)))))

If I understand this correctly, `install-file?' will be called with
`#:verbose #t' only when no files were installed. In that case, we would
simply get a long list of "excluded" statements. This seems a bit
redundant. Do we really need this?

... continued below

> +    (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?

> +          #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.