[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#41010] Upgrade Oil to 0.8.pre4
From: |
Tobias Geerinckx-Rice |
Subject: |
[bug#41010] Upgrade Oil to 0.8.pre4 |
Date: |
Sat, 02 May 2020 01:23:30 +0200 |
Ryan,
This patch upgrades the oil package. As noted in the package
description, upstream considers this to be a stable release &
the best available version of oil despite the "pre" tag.
Thanks!
I agree with the name change, although I'm unaware of why
‘oil-shell’ was originally chosen.
However, please do build and test patches before submitting them.
These were broken in 2 places: adding the unused ‘license:’ prefix
breaks evaluation, as does referring to a variable (‘oil’ in
deprecated-package) before it's defined.
Subject: [PATCH] gnu: oil: Update to 0.8.pre4
Add a full stop after commit summaries, and a ‘change log’ entry
as commit body:
* gnu/packages/shells.scm (oil): Update to 0.8.pre4.
+ (version "0.8.pre4") ; "Despite the pre4 version qualifier,
this is by far
+ ; the best Oil release ever… I may
change the version
+ ; numbering scheme in the near future
to reflect this."
+ ; - upstream on whether to ship pre4
in Guix
;; "Despite the pre4 version qualifier, this is by far
;; the best Oil release ever… I may change the version
;; numbering scheme in the near future to reflect this."
;; - upstream on whether to ship pre4 in Guix
(version "0.8.pre4")
Format long and/or multi-line comments like so:
OTOH a one-line link to that thread, if one exists, would be
preferable.
+ (source
+ (origin
+ (method url-fetch)
+ (uri (string-append
"https://www.oilshell.org/download/oil-"
+ version ".tar.gz"))
+ (sha256
+ (base32
+
"0m2p8p5hi2r14xx9pphsa0ar56kqsa33gr2w2blc3jx07aqhjpzy"))))
If you're going to re-indent like this, the hash fits beside
(base32 ….
- #:strip-binaries? #f ; the binaries cannot be stripped
+ `(#:strip-binaries? #f ; Strip breaks the binary.
I like your comment better but the original formatting (lowercase,
no full stop) was fine.
+ (setenv "CC" "gcc")
(let ((out (assoc-ref outputs "out")))
- (setenv "CC" "gcc")
(let ((out (assoc-ref outputs "out")))
(do-something "foo")
(do-something out))
It's canonical style in Guix (not sure about wider Schemeland) to
‘bind early’:
(do-something "foo")
(let ((out (assoc-ref outputs "out")))
(do-something out))
While you'll find a fair share of
it's much less common, possibly frowned upon, and idly rearranging
existing code is right out.
+ (substitute* "configure"
+ ((" cc ") " gcc "))
(substitute* "configure"
((" cc ") " $CC "))
+ (invoke
+ "./configure"
More line nitpicking: keep these on one line & indent the other
arguments accordingly.
+ (replace 'check ; The tests are not distributed in the
tarballs but
+ ; upstream recommends running this
smoke test.
Same as above:
(replace 'check
;; The tests are not distributed in the tarballs but upstream
;; recommends running this smoke test.
…
Where do they recommend this? It's nice to have a link in case
the recommendation changes.
+ (native-inputs
Nak. ‘Native’ means ‘when cross compiling, don't bother building
this for the target architecture, it will only ever run on the
host’…
`(("readline" ,readline)))
…as ‘guix gc --references oil’ (and readline's general nature)
tell us, that's not the case here: Oil links to it at run time, so
it must not be native.
- (synopsis "Bash-compatible Unix shell")
- (description "Oil is a Unix / POSIX shell, compatible with
Bash. It
-implements the Oil language, which is a new shell language to
which Bash can be
-automatically translated. The Oil language is a superset of
Bash. It also
-implements the OSH language, a statically-parseable language
based on Bash as it
-is commonly written.")
[…]
+ (synopsis "A Unix shell")
+ (description "Oil is a Unix shell and programming
language. It's our upgrade
+path from Bash.")
Both the original synopsis and description are much better. If
certain things are no longer accurate they can be adjusted but
this is just upstream's marketing pitch.
+ (license (list license:psfl ; Tarball vendors python2.7
Hmm, this doesn't parse as English (it's missing a verb). I'd
guess typo… but for what? Are upstream the ‘tarball vendors’
here? What was wrong with the original comment?
If upstream still bundles Python (and it would seem so), that's
important information that shouldn't be removed.
Phew. ‘I'll just review this trivial bump before bed-time.’ This
patch changes lots of things for one small package. I hope you
don't mind lots of comments :-)
Kind regards,
T G-R
signature.asc
Description: PGP signature
- [bug#41010] Rename & upgrade oil-shell, Ryan Prior, 2020/05/01
- [bug#41010] Upgrade Oil to 0.8.pre4, Ryan Prior, 2020/05/01
- [bug#41010] Upgrade Oil to 0.8.pre4,
Tobias Geerinckx-Rice <=
- [bug#41010] Upgrade Oil to 0.8.pre4, Tobias Geerinckx-Rice, 2020/05/01
- [bug#41010] Upgrade Oil to 0.8.pre4, Ryan Prior, 2020/05/02
- [bug#41010] Upgrade Oil to 0.8.pre4, Ryan Prior, 2020/05/15
- [bug#41010] Upgrade Oil to 0.8.pre4, Tobias Geerinckx-Rice, 2020/05/15
- [bug#41010] Upgrade Oil to 0.8.pre4, Tobias Geerinckx-Rice, 2020/05/16
- [bug#41010] Upgrade Oil to 0.8.pre4, Ryan Prior, 2020/05/17
- [bug#41010] Upgrade Oil to 0.8.pre4, Leo Famulari, 2020/05/02
[bug#41010] [PATCH 0/1] Updated patch for 0.8.pre5, Ryan Prior, 2020/05/28