Re: [PATCH] gnu: Add sassc.

[Thompson, David]

On Sat, Aug 22, 2015 at 12:57 AM, Mark H Weaver <address@hidden> wrote:
> David Thompson <address@hidden> writes:
>
>> From 4a6d1a798a0625d8bc104b86db56f5d2594a5d80 Mon Sep 17 00:00:00 2001
>> From: David Thompson <address@hidden>
>> Date: Wed, 19 Aug 2015 21:54:57 -0400
>> Subject: [PATCH] gnu: Add sassc.
>>
>> * gnu/packages/web.scm (sassc): New variable.
>> ---
>>  gnu/packages/web.scm | 53 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 53 insertions(+)
>>
>> diff --git a/gnu/packages/web.scm b/gnu/packages/web.scm
>> index 48bfbc7..ce4cfeb 100644
>> --- a/gnu/packages/web.scm
>> +++ b/gnu/packages/web.scm
>> @@ -642,6 +642,59 @@ minimum to provide high performance operation.")
>>      ;; bundled CuTest framework uses a different non-copyleft license.
>>      (license (list l:asl2.0 (l:non-copyleft 
>> "file://test/CuTest-README.txt")))))
>>
>> +(define-public sassc
>> +  ;; libsass must be statically linked and it isn't included in source the
>> +  ;; sassc release tarballs, hence this odd package recipe.
>> +  (let* ((version "3.2.5")
>> +         (libsass
>> +          (origin
>> +            (method url-fetch)
>> +            (uri (string-append
>> +                  "https://github.com/sass/libsass/archive/";
>> +                  version ".tar.gz"))
>> +            (file-name (string-append "libsass-" version ".tar.gz"))
>> +            (sha256
>> +             (base32
>> +              "1x25k6p1s1yzsdpzb7bzh8japilmi1mk3z96q66pycbinj9z9is4")))))
>> +    (package
>> +      (name "sassc")
>> +      (version version)
>> +      (source (origin
>> +                (method url-fetch)
>> +                (uri (string-append "https://github.com/sass/sassc/archive/";
>> +                                    version ".tar.gz"))
>
> Please add a 'file-name' field here.

Good catch.

>> +                (sha256
>> +                 (base32
>> +                  "1xf3w75w840rj0nx375rxi7mcv1ngqqq8p3zrzjlyx8jfpnldmv5"))))
>> +      (build-system gnu-build-system)
>> +      (arguments
>> +       `(#:make-flags '("CC=gcc")
>> +         #:test-target "test"
>> +         #:phases
>> +         (modify-phases %standard-phases
>> +           (delete 'configure)
>> +           (add-before 'build 'set-libsass-path
>> +             (lambda* (#:key inputs #:allow-other-keys)
>> +               (system* "tar" "xvf" (assoc-ref inputs "libsass"))
>
> I think this phase should be renamed, since it is doing much more than
> just setting the path.  How about 'unpack-libsass-and-set-path'?

Sounds good.  When I initially wrote the phase the name was accurate,
then I added the tarball unpacking step and forgot to change the name.

> The return value of 'system*' should be checked.

Done.

> Also, I think this pass should go after 'unpack' instead of before
> 'build', so that the other passes between 'unpack' and 'build' can do
> their jobs.  It's possible that it doesn't make a difference in this
> case, but that might change in a future version of libsass, and anyway
> since copy+paste from existing packages is a common method, we should
> encourage best practices.

Yes, good idea.

>> +               (setenv "SASS_LIBSASS_PATH"
>> +                       (string-append (getcwd) "/libsass-" ,version))
>> +
>> +               #t))
>
> I'd probably remove that blank line here.

Done.

>> +           (replace 'install ; no install target
>> +             (lambda* (#:key outputs #:allow-other-keys)
>> +               (let ((bin (string-append (assoc-ref outputs "out") "/bin")))
>> +                 (mkdir-p bin)
>> +                 (copy-file "bin/sassc"
>> +                            (string-append bin "/sassc"))
>> +                 #t))))))
>> +      (inputs
>> +       `(("libsass" ,libsass)))
>> +      (synopsis "CSS pre-processor")
>> +      (description "SassC is a compiler written in C for the CSS 
>> pre-processor
>> +language known as SASS.")
>> +      (home-page "http://sass-lang.com/libsass";)
>> +      (license l:expat))))
>
> Otherwise it looks good to me.  Thanks!

Pushed, thanks!

- Dave