[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] gnu: Add p7zip.
From: |
Kei Kebreau |
Subject: |
Re: [PATCH] gnu: Add p7zip. |
Date: |
Sat, 27 Aug 2016 06:36:01 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Kei Kebreau <address@hidden> writes:
> Ricardo Wurmus <address@hidden> writes:
>
>> Kei Kebreau <address@hidden> 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.
> Attached is a patch with your adjustments taken into consideration.
>
>>
>> Thanks for your patience so far :)
>>
>> ~~ Ricardo
> You're welcome! I expect that you're busy and getting to things as time
> and energy allows. :)
Whoops, last patch was a bit messy and stacked on the previous one. This
patch should be better!
0001-gnu-Add-p7zip.patch
Description: Text document
signature.asc
Description: PGP signature
- Re: [PATCH] gnu: Add p7zip., (continued)
- Re: [PATCH] gnu: Add p7zip., Eric Bavier, 2016/08/02
- Re: [PATCH] gnu: Add p7zip., Leo Famulari, 2016/08/08
- Re: [PATCH] gnu: Add p7zip., Ricardo Wurmus, 2016/08/09
- Re: [PATCH] gnu: Add p7zip., kei, 2016/08/09
- Re: [PATCH] gnu: Add p7zip., Ricardo Wurmus, 2016/08/09
- Re: [PATCH] gnu: Add p7zip., Kei Kebreau, 2016/08/09
- Re: [PATCH] gnu: Add p7zip., Ricardo Wurmus, 2016/08/09
- Re: [PATCH] gnu: Add p7zip., Kei Kebreau, 2016/08/12
- Re: [PATCH] gnu: Add p7zip., Ricardo Wurmus, 2016/08/22
- Re: [PATCH] gnu: Add p7zip., Kei Kebreau, 2016/08/22
- Re: [PATCH] gnu: Add p7zip.,
Kei Kebreau <=
- Re: [PATCH] gnu: Add p7zip., Ricardo Wurmus, 2016/08/31