Kei Kebreau <k...@openmailbox.org> writes:
> Here's the corresponding patch. Maybe you or someone else can double (or
> triple?)
> check and make sure there are no proprietary files the source after running
> "./pre-inst-env guix build -S p7zip."
The patch looks good. It’s fine to delete the non-free files in a
snippet. I have a couple of changes that I’d to see in the package
expression, though.
> +
> +(define-public p7zip
> + (package
> + (name "p7zip")
> + (version "16.02")
> + (source (origin
> + (method url-fetch)
> + (uri (string-append "mirror://sourceforge/" name "/" name "/"
> + version "/" name "_" version
> + "_src_all.tar.bz2"))
> + (sha256
> + (base32
> + "07rlwbbgszq8i7m8jh3x6j2w2hc9a72dc7fmqawnqkwlwb00mcjy"))
> + (modules '((guix build utils)))
> + (snippet
> + '(begin
> + ;; Remove non-free source files
> + (for-each delete-file
> + (find-files "CPP/7zip/Compress/"
> + (string-append "Rar*")))
You don’t need “string-append” here.
> + (delete-file-recursively "CPP/7zip/Compress/Rar")))
I think we may want to delete even more Rar stuff to be on the safe
side, so the total list of deletions would be:
CPP/7zip/Archive/Rar/
CPP/7zip/Archive/Rar*
CPP/7zip/Crypto/Rar*
CPP/7zip/Compress/Rar*
DOC/unRarLicense.txt
Utils/file_Codecs_Rar_so.py
What do you think?
Also note that the snippet should end on “#t”.
> + (patches (search-patches "remove-unused-p7zip-code.patch"))))
> + (build-system gnu-build-system)
> + (arguments
> + `(#:make-flags
> + (list (let ((system ,(or (%current-target-system)
> + (%current-system))))
> + (string-append "-f "
> + (cond
> + ((string-prefix? "x86_64" system)
> + "makefile.linux_amd64_asm")
> + ((string-prefix? "i686" system)
> + "makefile.linux_x86_asm_gcc_4.X")
> + (else
> + "makefile.linux_any_cpu_gcc_4.X")))))
> + #:phases
> + (modify-phases %standard-phases
> + (replace 'configure
> + (lambda* (#:key system outputs #:allow-other-keys)
> + ;; fix install directory
> + (substitute* "install.sh"
> + (("/usr/local") (assoc-ref outputs "out")))))
We should end phases with “#t”. However, in this case I think we should
better move this to the replaced install phase to keep related things in
one place.
> + (replace 'build
> + (lambda _
> + (zero? (system* "make" "all3"))))
I think we can avoid replacing the build phase by adding
#:make-flags '("all3")
instead. What do you think?
> + (replace 'check
> + (lambda _
> + (and (zero? (system* "make" "test"))
> + (zero? (system* "make" "test_7z"))
> + (zero? (system* "make" "test_7zr")))))
> + ;; without replacing the install phase, install.sh would be passed
> + ;; arguments containing the wrong installation directories.
> + (replace 'install
> + (lambda _
> + (zero? (system* "sh" "install.sh")))))))
I don’t understand the comment. The makefiles don’t seem to have an
“install” target at all.
> + (inputs
> + `(,@(cond ((string-prefix? "x86_64" (or (%current-target-system)
> + (%current-system)))
> + `(("yasm" ,yasm)))
> + ((string-prefix? "i686" (or (%current-target-system)
> + (%current-system)))
> + `(("nasm" ,nasm)))
> + (else '()))))
> + (home-page "http://p7zip.sourceforge.net/";)
> + (synopsis "Command-line file archiver with high compression ratio")
> + (description "p7zip is the Unix command-line port of 7-Zip, a file
> archiver
> +that handles the 7z format which features very high compression
> ratios.")
How about replacing “Unix” with “POSIX”?
> + (license (list license:lgpl2.1+ license:gpl2+
> license:public-domain))))
Here I’d like to see a comment that explains what this list means.
Do you think you could provide us with an updated patch? If you’re
already tired of this patch I could perform the changes on your behalf
some time later.
Thanks for your patience so far :)
~~ Ricardo
