unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* (unknown), 
@ 2016-12-19 17:50 John Darrington
  2016-12-19 17:50 ` [PATCH] gnu: Fix load-extension path in packaging of guile-ncurses John Darrington
  0 siblings, 1 reply; 13+ messages in thread
From: John Darrington @ 2016-12-19 17:50 UTC (permalink / raw)
  To: guix-devel


Thanks to Ludovic for showing me how to fix this.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH] gnu: Fix load-extension path in packaging of guile-ncurses.
  2016-12-19 17:50 (unknown), John Darrington
@ 2016-12-19 17:50 ` John Darrington
  2016-12-19 21:17   ` Ludovic Courtès
  2016-12-20  7:17   ` Mark H Weaver
  0 siblings, 2 replies; 13+ messages in thread
From: John Darrington @ 2016-12-19 17:50 UTC (permalink / raw)
  To: guix-devel; +Cc: John Darrington

* gnu/packages/guile.scm (guile-ncurses) [arguments]: Install shared object before
attempting to build the package.  Patch load-extension path before building instead
of after.
---
 gnu/packages/guile.scm | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/gnu/packages/guile.scm b/gnu/packages/guile.scm
index 1653685..741728d 100644
--- a/gnu/packages/guile.scm
+++ b/gnu/packages/guile.scm
@@ -431,16 +431,20 @@ many readers as needed).")
                                "--with-gnu-filesystem-hierarchy")
        #:phases
        (modify-phases %standard-phases
-         (add-after 'install 'post-install
+         (add-before 'build 'fix-libguile-ncurses-file-name
            (lambda* (#:key outputs #:allow-other-keys)
-             (let* ((out   (assoc-ref outputs "out"))
-                    (dir   (string-append out "/share/guile/site/"))
-                    (files (find-files dir ".scm")))
-               (substitute* files
-                 (("\"libguile-ncurses\"")
-                  (format #f "\"~a/lib/guile/2.0/libguile-ncurses\""
-                          out)))
-               #t))))))
+             (and (zero? (system* "make" "install"
+                                  "-C" "src/ncurses"
+                                  "-j" (number->string
+                                        (parallel-job-count))))
+                  (let* ((out   (assoc-ref outputs "out"))
+                         (dir   "src/ncurses")
+                         (files (find-files dir ".scm")))
+                    (substitute* files
+                      (("\"libguile-ncurses\"")
+                       (format #f "\"~a/lib/guile/2.0/libguile-ncurses\""
+                               out)))
+                    #t)))))))
     (home-page "https://www.gnu.org/software/guile-ncurses/")
     (synopsis "Guile bindings to ncurses")
     (description
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: Fix load-extension path in packaging of guile-ncurses.
  2016-12-19 17:50 ` [PATCH] gnu: Fix load-extension path in packaging of guile-ncurses John Darrington
@ 2016-12-19 21:17   ` Ludovic Courtès
  2016-12-20  7:17   ` Mark H Weaver
  1 sibling, 0 replies; 13+ messages in thread
From: Ludovic Courtès @ 2016-12-19 21:17 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel

John Darrington <jmd@gnu.org> skribis:

> * gnu/packages/guile.scm (guile-ncurses) [arguments]: Install shared object before
> attempting to build the package.  Patch load-extension path before building instead
> of after.

OK, thank you!

Ludo’.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: Fix load-extension path in packaging of guile-ncurses.
  2016-12-19 17:50 ` [PATCH] gnu: Fix load-extension path in packaging of guile-ncurses John Darrington
  2016-12-19 21:17   ` Ludovic Courtès
@ 2016-12-20  7:17   ` Mark H Weaver
  2016-12-20 11:03     ` John Darrington
  1 sibling, 1 reply; 13+ messages in thread
From: Mark H Weaver @ 2016-12-20  7:17 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel

Hi John,

John Darrington <jmd@gnu.org> writes:
> * gnu/packages/guile.scm (guile-ncurses) [arguments]: Install shared object before
> attempting to build the package.  Patch load-extension path before building instead
> of after.

The first sentence above is mistaken or misleading:

  "Install shared object before attempting to build the package."

Of course this is not possible, and it's not what happens after applying
this commit.

> +         (add-before 'build 'fix-libguile-ncurses-file-name
>             (lambda* (#:key outputs #:allow-other-keys)
> - [...]
> +             (and (zero? (system* "make" "install"
> +                                  "-C" "src/ncurses"
> +                                  "-j" (number->string
> +                                        (parallel-job-count))))
> +                  (let* ((out   (assoc-ref outputs "out"))
> +                         (dir   "src/ncurses")
> +                         (files (find-files dir ".scm")))
> +                    (substitute* files
> +                      (("\"libguile-ncurses\"")
> +                       (format #f "\"~a/lib/guile/2.0/libguile-ncurses\""
> +                               out)))
> +                    #t)))))))

By running "make install -C src/ncurses [...]", you are in fact causing
most if not all of the package to be built and installed right here in
the 'fix-libguile-ncurses-file-name' phase.  This is no good.

Can you help me understand what you're trying to do here, and why?

     Thanks,
       Mark

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: Fix load-extension path in packaging of guile-ncurses.
  2016-12-20  7:17   ` Mark H Weaver
@ 2016-12-20 11:03     ` John Darrington
  2016-12-20 14:16       ` John Darrington
  2016-12-21  8:36       ` Danny Milosavljevic
  0 siblings, 2 replies; 13+ messages in thread
From: John Darrington @ 2016-12-20 11:03 UTC (permalink / raw)
  To: Mark H Weaver; +Cc: guix-devel, John Darrington

[-- Attachment #1: Type: text/plain, Size: 2965 bytes --]

On Tue, Dec 20, 2016 at 02:17:42AM -0500, Mark H Weaver wrote:
     Hi John,
     
     John Darrington <jmd@gnu.org> writes:
     > * gnu/packages/guile.scm (guile-ncurses) [arguments]: Install shared object before
     > attempting to build the package.  Patch load-extension path before building instead
     > of after.
     
     The first sentence above is mistaken or misleading:
     
       "Install shared object before attempting to build the package."

How about I change it to "... attempting to build the rest of the package" ??

     > +         (add-before 'build 'fix-libguile-ncurses-file-name
     >             (lambda* (#:key outputs #:allow-other-keys)
     > - [...]
     > +             (and (zero? (system* "make" "install"
     > +                                  "-C" "src/ncurses"
     > +                                  "-j" (number->string
     > +                                        (parallel-job-count))))
     > +                  (let* ((out   (assoc-ref outputs "out"))
     > +                         (dir   "src/ncurses")
     > +                         (files (find-files dir ".scm")))
     > +                    (substitute* files
     > +                      (("\"libguile-ncurses\"")
     > +                       (format #f "\"~a/lib/guile/2.0/libguile-ncurses\""
     > +                               out)))
     > +                    #t)))))))
     
     By running "make install -C src/ncurses [...]", you are in fact causing
     most if not all of the package to be built and installed right here in
     the 'fix-libguile-ncurses-file-name' phase.

I didn't check to see what percentage of the package is contained in the 
src/ncurses directory, but it certainly isn't "all".

     This is no good.

Why do you think it is "no good"?  It may not be perfect, but it is better
than what we had before, and certainly fixes the problem.

     
     Can you help me understand what you're trying to do here, and why?
     
Sure (I would like to see a convention where such explanations are
put in the commit messaage, but I have previously been outvoted on
that issue):

The scheme code contains a number of procedures similar to 
(load-extension "libguile-ncurses" "func"). We need the first
string to contain the absolute path where the shared object is 
installed.   The previous version had done this by patching the 
.scm file post-installation.  However that meant that the compiled
.go file was compiled with the old, unmodified version.  
This patch ensures that the .scm files are patched first, and that
the .go files are built from the patched .scm files.

I hope this makes things clearer for you.

J'


-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: Fix load-extension path in packaging of guile-ncurses.
  2016-12-20 11:03     ` John Darrington
@ 2016-12-20 14:16       ` John Darrington
  2016-12-21  8:36       ` Danny Milosavljevic
  1 sibling, 0 replies; 13+ messages in thread
From: John Darrington @ 2016-12-20 14:16 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel, John Darrington

[-- Attachment #1: Type: text/plain, Size: 1027 bytes --]

On Tue, Dec 20, 2016 at 12:03:31PM +0100, John Darrington wrote:
     On Tue, Dec 20, 2016 at 02:17:42AM -0500, Mark H Weaver wrote:

     The scheme code contains a number of procedures similar to 
     (load-extension "libguile-ncurses" "func"). We need the first
     string to contain the absolute path where the shared object is 
     installed.   The previous version had done this by patching the 

      ^^^ 
For clarification, when I say "installed" here,  I mean it's location 
in /gnu/store.

     .scm file post-installation.  However that meant that the compiled
     .go file was compiled with the old, unmodified version.  
     This patch ensures that the .scm files are patched first, and that
     the .go files are built from the patched .scm files.
     



-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: Fix load-extension path in packaging of guile-ncurses.
  2016-12-20 11:03     ` John Darrington
  2016-12-20 14:16       ` John Darrington
@ 2016-12-21  8:36       ` Danny Milosavljevic
  2016-12-21  9:56         ` John Darrington
  1 sibling, 1 reply; 13+ messages in thread
From: Danny Milosavljevic @ 2016-12-21  8:36 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel, John Darrington

> Sure (I would like to see a convention where such explanations are
> put in the commit messaage, but I have previously been outvoted on
> that issue):

No, please don't put explanations into the commit message. But do put them into the source code as a comment.

I'm also working on other projects, some of which do what you propose. What I often end up having to do there is do git blame, then git log for each line, in order to find out why the source code does what it does. Let's not do that here. There's a perfectly good inline mechanism for it: Comments.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: Fix load-extension path in packaging of guile-ncurses.
  2016-12-21  8:36       ` Danny Milosavljevic
@ 2016-12-21  9:56         ` John Darrington
  2016-12-22  5:56           ` Tobias Geerinckx-Rice
  0 siblings, 1 reply; 13+ messages in thread
From: John Darrington @ 2016-12-21  9:56 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 3710 bytes --]

On Wed, Dec 21, 2016 at 09:36:56AM +0100, Danny Milosavljevic wrote:
     > Sure (I would like to see a convention where such explanations are
     > put in the commit messaage, but I have previously been outvoted on
     > that issue):

Hi Danny, 

A small request: Can you please fold the text of your email to ~80 
characters.  It's very hard to read otherwise.
     
     No, please don't put explanations into the commit message. But do put them into the source code as a comment.

That approach can work sometimes, but more often it is a non-starter.

A few example scenarios:

1. Suppose I need to do a global search and replace, changing a variable name 
across many files.  It would be ludicrous  to have in dozens of files:

;; This variable used to be called "bar" but we changed it to "foo" because ...
(+ foo 4)

When reviewing the code, frankly nobody CARES what it used to be called!


2. Suppose we decide to delete something from a file. It would be equally 
ludicrous to have:

;; There used to be some code here which did:
;;(large-block-of-code
;;.
;;.
;;.
;;.
;;.
;;.)
;; but we decided to delete it because ...

Again I don't care what used to be in a file.


3.  Suppose that, due to a design change, a new variable has to be 
introduced in places thoughout the code:  It would be bizarre, 
distracting and stupid to have in many places:

;; Since we introduced the frobnicator module the signature for
;; calling wiz needs to pass it as an argument.
(wiz frobnicator)



4. Suppose I fix a bug:  It would be pejorative to write:

;; Fred Bloggs who wrote the function typed 'xyz' when he
;; ought to have put 'abc', because ...


I am just glad when a bug has been fixed.  If somebody changes
some code of mine, I might be curious as to why.  In that case
I can check the git log.  If that person has (like he should)
explained in the git log why my code was wrong, I will be 
gratefull for the explanation and the fix.  But nobody except
me will care about bugs in the function which have been fixed.


     I'm also working on other projects, some of which do what you propose. What I often end up having to do there is do git blame, then git log for each line, in order to find out why the source code does what it does. Let's not do that here. 

That is what git blame is for.  Be thankful for it!

  There's a perfectly good inline mechanism for it: Comments.

I am not saying that no explanations of *current* code should 
be put in comments.  It is of course good practice to explain
the working of tricky parts of code.  But to put a *history*
of the code inline is just distraction and a misuse of comments.


Your proposal takes us back to the 1970s - Occassionally I come across
code done like that.  It is  a nightmare to follow.  I am not normally
interested in the history of the code when I look a the source.  I am 
interested in what it does now.   If I want the history, then use git.
That is (amoung other things) what it was designed for.


I hope this explains why putting history in comments is harmful.
Having it in the commit message would certainly have avoided me
having to explain the situation to Mark too.


If this doesn't convince you, then I don't know what more I can say.
But I find that our current git logs are just useless.  They don't
tell me anything which I couldn't have found out by running 
git diff/git blame.

J'


-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: Fix load-extension path in packaging of guile-ncurses.
  2016-12-21  9:56         ` John Darrington
@ 2016-12-22  5:56           ` Tobias Geerinckx-Rice
  2016-12-22  8:20             ` John Darrington
  0 siblings, 1 reply; 13+ messages in thread
From: Tobias Geerinckx-Rice @ 2016-12-22  5:56 UTC (permalink / raw)
  To: john; +Cc: guix-devel


[-- Attachment #1.1: Type: text/plain, Size: 3998 bytes --]

John, Danny,

[Any exasperation is due only to the sustained level of FUD I encounter
about the Guix/GNU changelog format, and not aimed at John.]

On 20/12/16 12:03, John Darrington wrote:
> Sure (I would like to see a convention where such explanations are 
> put in the commit messaage, but I have previously been outvoted on 
> that issue).

I don't think so.

1. What was ‘outvoted’ (with solid argumentation) was hiding comments
documenting code itself in commit messages, when they would do more
good as, well, comments.

2. The Guix commit log has plenty of concise explanations for why code
was *changed*. I See 64b5e41 for a random example. If that guy can get
away with it...

Most People™ badly overdo #2 when they should do #1 (consider the
average *Hub pull request). The opposite is less common.

> On Wed, Dec 21, 2016 at 09:36:56AM +0100, Danny Milosavljevic wrote:
>> No, please don't put explanations into the commit message. But do 
>> put them into the source code as a comment.

I'd just finished writing the exact same e-mail as Danny — almost to the
sentence — when that arrived. So... it must be right! :-)

> That approach can work sometimes, but more often it is a 
> non-starter.

[Examples paraphrased for length:]
> 1. ;; This variable used to be called "bar" but we changed it
> 2. ;; There used to be some code here but we deleted it
> 3. A new variable was introduced in places thoughout the code
> 4. ;; Fred typed 'xyz' when he ought to have put 'abc'

Sorry, but the first three examples are silly. :-)

Deliberately silly, I'm sure, but still these are strawmen that no-one
proposed. On the contrary: examples 1 to 3 are exactly what the current
commit log documents with tedious precision.

That leaves bugfixes.

If the bug is waiting to happen again (and again...), putting a warning
to future readers in a comment might be appropriate. No-one will spot it
in a commit message. (Of course, re-writing the misleading/dangerous
code would be the best solution.)

If the fix is trivial, as in this example, the diff usually speaks for
itself. In all other cases, a short note in the commit message is fine.
This seems to be standard policy for CVEs. So I really don't understand
your complaint.

> But nobody except me will care about bugs in the function which have
> been fixed.

Exactly! In this patch, you're replacing buggy (?) code with shiny code.
That shouldn't take more than ~50 characters to note.

> On Wed, Dec 21, 2016 at 09:36:56AM +0100, Danny Milosavljevic wrote:
>> I'm also working on other projects, some of which do what you 
>> propose. What I often end up having to do there is do git blame, 
>> then git log for each line, in order to find out why the source 
>> code does what it does. Let's not do that here.

+11, unfortunately from experience.

> That is what git blame is for.  Be thankful for it!

No and God no.

That is exactly what ‘git blame’ is not for.

Code itself should be documented in-line. Not in a commit log meant for
documenting changes, that breaks as soon as someone hits C-M-q.

> I hope this explains why putting history in comments is harmful.

It does! But this is not something anyone here suggested.

> Having it in the commit message would certainly have avoided me 
> having to explain the situation to Mark too.

Perhaps. I doubt it. I can't speak for Mark, but most confusion
seemed to stem from the commit message's accuracy, not its length.

But yes, it could have been longer — and a comment :-)

Kind regards,

T G-R

PS: On 21/12/16 10:56, John Darrington wrote:
> Hi Danny,
> 
> A small request: Can you please fold the text of your email to ~80 
> characters.  It's very hard to read otherwise.

FWIW, your replies are equally hard to read (and even harder to reply
to) because of the unconventional indentation. It certainly confuses
Thunderbird, which is easily confused.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 476 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: Fix load-extension path in packaging of guile-ncurses.
  2016-12-22  5:56           ` Tobias Geerinckx-Rice
@ 2016-12-22  8:20             ` John Darrington
  2016-12-24 15:39               ` Mark H Weaver
  0 siblings, 1 reply; 13+ messages in thread
From: John Darrington @ 2016-12-22  8:20 UTC (permalink / raw)
  To: Tobias Geerinckx-Rice; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 5104 bytes --]

We can argue about this till we're blue in the face.

But on a pragmatic level, Mark's question demonstrates perfectly
that our current system is lacking.  Other projects I work on
which have a more conventional approach do not suffer from this problem.

J'

On Thu, Dec 22, 2016 at 06:56:56AM +0100, Tobias Geerinckx-Rice wrote:
     John, Danny,
     
     [Any exasperation is due only to the sustained level of FUD I encounter
     about the Guix/GNU changelog format, and not aimed at John.]
     
     On 20/12/16 12:03, John Darrington wrote:
     > Sure (I would like to see a convention where such explanations are 
     > put in the commit messaage, but I have previously been outvoted on 
     > that issue).
     
     I don't think so.
     
     1. What was ???outvoted??? (with solid argumentation) was hiding comments
     documenting code itself in commit messages, when they would do more
     good as, well, comments.
     
     2. The Guix commit log has plenty of concise explanations for why code
     was *changed*. I See 64b5e41 for a random example. If that guy can get
     away with it...
     
     Most People??? badly overdo #2 when they should do #1 (consider the
     average *Hub pull request). The opposite is less common.
     
     > On Wed, Dec 21, 2016 at 09:36:56AM +0100, Danny Milosavljevic wrote:
     >> No, please don't put explanations into the commit message. But do 
     >> put them into the source code as a comment.
     
     I'd just finished writing the exact same e-mail as Danny ??? almost to the
     sentence ??? when that arrived. So... it must be right! :-)
     
     > That approach can work sometimes, but more often it is a 
     > non-starter.
     
     [Examples paraphrased for length:]
     > 1. ;; This variable used to be called "bar" but we changed it
     > 2. ;; There used to be some code here but we deleted it
     > 3. A new variable was introduced in places thoughout the code
     > 4. ;; Fred typed 'xyz' when he ought to have put 'abc'
     
     Sorry, but the first three examples are silly. :-)
     
     Deliberately silly, I'm sure, but still these are strawmen that no-one
     proposed. On the contrary: examples 1 to 3 are exactly what the current
     commit log documents with tedious precision.
     
     That leaves bugfixes.
     
     If the bug is waiting to happen again (and again...), putting a warning
     to future readers in a comment might be appropriate. No-one will spot it
     in a commit message. (Of course, re-writing the misleading/dangerous
     code would be the best solution.)
     
     If the fix is trivial, as in this example, the diff usually speaks for
     itself. In all other cases, a short note in the commit message is fine.
     This seems to be standard policy for CVEs. So I really don't understand
     your complaint.
     
     > But nobody except me will care about bugs in the function which have
     > been fixed.
     
     Exactly! In this patch, you're replacing buggy (?) code with shiny code.
     That shouldn't take more than ~50 characters to note.
     
     > On Wed, Dec 21, 2016 at 09:36:56AM +0100, Danny Milosavljevic wrote:
     >> I'm also working on other projects, some of which do what you 
     >> propose. What I often end up having to do there is do git blame, 
     >> then git log for each line, in order to find out why the source 
     >> code does what it does. Let's not do that here.
     
     +11, unfortunately from experience.
     
     > That is what git blame is for.  Be thankful for it!
     
     No and God no.
     
     That is exactly what ???git blame??? is not for.
     
     Code itself should be documented in-line. Not in a commit log meant for
     documenting changes, that breaks as soon as someone hits C-M-q.
     
     > I hope this explains why putting history in comments is harmful.
     
     It does! But this is not something anyone here suggested.
     
     > Having it in the commit message would certainly have avoided me 
     > having to explain the situation to Mark too.
     
     Perhaps. I doubt it. I can't speak for Mark, but most confusion
     seemed to stem from the commit message's accuracy, not its length.
     
     But yes, it could have been longer ??? and a comment :-)
     
     Kind regards,
     
     T G-R
     
     PS: On 21/12/16 10:56, John Darrington wrote:
     > Hi Danny,
     > 
     > A small request: Can you please fold the text of your email to ~80 
     > characters.  It's very hard to read otherwise.
     
     FWIW, your replies are equally hard to read (and even harder to reply
     to) because of the unconventional indentation. It certainly confuses
     Thunderbird, which is easily confused.
     




-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: Fix load-extension path in packaging of guile-ncurses.
  2016-12-22  8:20             ` John Darrington
@ 2016-12-24 15:39               ` Mark H Weaver
  2016-12-24 17:02                 ` John Darrington
  0 siblings, 1 reply; 13+ messages in thread
From: Mark H Weaver @ 2016-12-24 15:39 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel

John Darrington <john@darrington.wattle.id.au> writes:

> We can argue about this till we're blue in the face.
>
> But on a pragmatic level, Mark's question demonstrates perfectly
> that our current system is lacking.

No it doesn't.  Our convention, taken from the GNU coding standards, is
that the rationale for non-obvious code belongs in the code itself.  My
question demonstrates perfectly that you should have done _that_.

For what it's worth, I agree that there are some cases where adding
rationale comments to the code itself doesn't make sense (e.g. when
removing code), but this is clearly not one of those cases.

>      > Having it in the commit message would certainly have avoided me 
>      > having to explain the situation to Mark too.
>      
>      Perhaps. I doubt it. I can't speak for Mark, but most confusion
>      seemed to stem from the commit message's accuracy, not its length.

Yes, exactly.

To be honest, I find it unsettling that after all that has been pointed
out in this thread, you still seem unwilling to admit that you made any
mistake here.

Have you looked at the build log, and specifically the part of the build
log that corresponds to your 'fix-libguile-ncurses-file-name' phase?

Have you noticed how the 'build' and 'install' phases consist mostly of
commands that were already run in your custom phase?

Do you still think that "Install shared object before attempting to
build the package" is an accurate statement?

       Mark

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: Fix load-extension path in packaging of guile-ncurses.
  2016-12-24 15:39               ` Mark H Weaver
@ 2016-12-24 17:02                 ` John Darrington
  2017-01-16  0:11                   ` Mark H Weaver
  0 siblings, 1 reply; 13+ messages in thread
From: John Darrington @ 2016-12-24 17:02 UTC (permalink / raw)
  To: Mark H Weaver; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 2090 bytes --]

On Sat, Dec 24, 2016 at 10:39:40AM -0500, Mark H Weaver wrote:
     John Darrington <john@darrington.wattle.id.au> writes:
     
     > We can argue about this till we're blue in the face.
     >
     > But on a pragmatic level, Mark's question demonstrates perfectly
     > that our current system is lacking.
     
     No it doesn't.  Our convention, taken from the GNU coding standards, is
     that the rationale for non-obvious code belongs in the code itself.  My
     question demonstrates perfectly that you should have done _that_.
     
     For what it's worth, I agree that there are some cases where adding
     rationale comments to the code itself doesn't make sense (e.g. when
     removing code), but this is clearly not one of those cases.
     
     >      > Having it in the commit message would certainly have avoided me 
     >      > having to explain the situation to Mark too.
     >      
     >      Perhaps. I doubt it. I can't speak for Mark, but most confusion
     >      seemed to stem from the commit message's accuracy, not its length.
     
     Yes, exactly.
     
     To be honest, I find it unsettling that after all that has been pointed
     out in this thread, you still seem unwilling to admit that you made any
     mistake here.
     
     Have you looked at the build log, and specifically the part of the build
     log that corresponds to your 'fix-libguile-ncurses-file-name' phase?
     
     Have you noticed how the 'build' and 'install' phases consist mostly of
     commands that were already run in your custom phase?
     
     Do you still think that "Install shared object before attempting to
     build the package" is an accurate statement?
     
I offered to change this comment.  You have ignored my offer.  Why are you 
determined to start an argument?


-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: Fix load-extension path in packaging of guile-ncurses.
  2016-12-24 17:02                 ` John Darrington
@ 2017-01-16  0:11                   ` Mark H Weaver
  0 siblings, 0 replies; 13+ messages in thread
From: Mark H Weaver @ 2017-01-16  0:11 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel

John Darrington <john@darrington.wattle.id.au> writes:

> On Sat, Dec 24, 2016 at 10:39:40AM -0500, Mark H Weaver wrote:
>      John Darrington <john@darrington.wattle.id.au> writes:
>      
>      > We can argue about this till we're blue in the face.
>      >
>      > But on a pragmatic level, Mark's question demonstrates perfectly
>      > that our current system is lacking.
>      
>      No it doesn't.  Our convention, taken from the GNU coding standards, is
>      that the rationale for non-obvious code belongs in the code itself.  My
>      question demonstrates perfectly that you should have done _that_.
>      
>      For what it's worth, I agree that there are some cases where adding
>      rationale comments to the code itself doesn't make sense (e.g. when
>      removing code), but this is clearly not one of those cases.
>      
>      >      > Having it in the commit message would certainly have avoided me 
>      >      > having to explain the situation to Mark too.
>      >      
>      >      Perhaps. I doubt it. I can't speak for Mark, but most confusion
>      >      seemed to stem from the commit message's accuracy, not its length.
>      
>      Yes, exactly.
>      
>      To be honest, I find it unsettling that after all that has been pointed
>      out in this thread, you still seem unwilling to admit that you made any
>      mistake here.
>      
>      Have you looked at the build log, and specifically the part of the build
>      log that corresponds to your 'fix-libguile-ncurses-file-name' phase?
>      
>      Have you noticed how the 'build' and 'install' phases consist mostly of
>      commands that were already run in your custom phase?
>      
>      Do you still think that "Install shared object before attempting to
>      build the package" is an accurate statement?
>      
> I offered to change this comment.  You have ignored my offer.

It's impossible to change that comment, because it's in the commit log.
That's yet another reason why the source code itself is a better place
for documentation than the commit logs.

      Mark

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-01-16  0:12 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-19 17:50 (unknown), John Darrington
2016-12-19 17:50 ` [PATCH] gnu: Fix load-extension path in packaging of guile-ncurses John Darrington
2016-12-19 21:17   ` Ludovic Courtès
2016-12-20  7:17   ` Mark H Weaver
2016-12-20 11:03     ` John Darrington
2016-12-20 14:16       ` John Darrington
2016-12-21  8:36       ` Danny Milosavljevic
2016-12-21  9:56         ` John Darrington
2016-12-22  5:56           ` Tobias Geerinckx-Rice
2016-12-22  8:20             ` John Darrington
2016-12-24 15:39               ` Mark H Weaver
2016-12-24 17:02                 ` John Darrington
2017-01-16  0:11                   ` Mark H Weaver

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).