GNU bug report logs - #25882
ld-wrapper does not handle response files

Previous Next

Package: guix;

Reported by: Federico Beffa <beffa <at> fbengineering.ch>

Date: Sun, 26 Feb 2017 17:39:02 UTC

Severity: normal

Tags: patch

Done: ludo <at> gnu.org (Ludovic Courtès)

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 25882 in the body.
You can then email your comments to 25882 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-guix <at> gnu.org:
bug#25882; Package guix. (Sun, 26 Feb 2017 17:39:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Federico Beffa <beffa <at> fbengineering.ch>:
New bug report received and forwarded. Copy sent to bug-guix <at> gnu.org. (Sun, 26 Feb 2017 17:39:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Federico Beffa <beffa <at> fbengineering.ch>
To: bug-guix <at> gnu.org
Subject: gcc-wrapper doesn't handle response files
Date: Sun, 26 Feb 2017 17:58:02 +0100
gcc-wrapper doesn't handle compiler/linker flags passed through
response files.

One package which recently started using such files is GHC (I believe
since 7.10.3).  For this reason we currently need to patch it.
However, the problem is with our tool chain wrapper and not with GHC
itself.

See discussion at
https://lists.gnu.org/archive/html/guix-devel/2017-01/msg01981.html




Information forwarded to bug-guix <at> gnu.org:
bug#25882; Package guix. (Mon, 06 Mar 2017 16:23:01 GMT) Full text and rfc822 format available.

Message #8 received at 25882 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Federico Beffa <beffa <at> fbengineering.ch>
Cc: 25882 <at> debbugs.gnu.org
Subject: Re: bug#25882: gcc-wrapper doesn't handle response files
Date: Mon, 06 Mar 2017 17:22:38 +0100
Federico Beffa <beffa <at> fbengineering.ch> skribis:

> gcc-wrapper doesn't handle compiler/linker flags passed through
> response files.
>
> One package which recently started using such files is GHC (I believe
> since 7.10.3).  For this reason we currently need to patch it.
> However, the problem is with our tool chain wrapper and not with GHC
> itself.
>
> See discussion at
> https://lists.gnu.org/archive/html/guix-devel/2017-01/msg01981.html

Given that the GHC patch is so small, I have a slight preference for
keeping ld-wrapper unchanged and using the GHC patch.  To put it
differently, the GHC patch is smaller and less error-prone than the
changes that would need to be made in ld-wrapper.

WDYT?

Thanks,
Ludo’.




Information forwarded to bug-guix <at> gnu.org:
bug#25882; Package guix. (Mon, 06 Mar 2017 17:17:01 GMT) Full text and rfc822 format available.

Message #11 received at 25882 <at> debbugs.gnu.org (full text, mbox):

From: Federico Beffa <beffa <at> ieee.org>
To: ludo <at> gnu.org (Ludovic Courtès)
Cc: 25882 <at> debbugs.gnu.org
Subject: Re: bug#25882: gcc-wrapper doesn't handle response files
Date: Mon, 06 Mar 2017 18:16:50 +0100
ludo <at> gnu.org (Ludovic Courtès) writes:

> Federico Beffa <beffa <at> fbengineering.ch> skribis:
>
>> gcc-wrapper doesn't handle compiler/linker flags passed through
>> response files.
>>
>> One package which recently started using such files is GHC (I believe
>> since 7.10.3).  For this reason we currently need to patch it.
>> However, the problem is with our tool chain wrapper and not with GHC
>> itself.
>>
>> See discussion at
>> https://lists.gnu.org/archive/html/guix-devel/2017-01/msg01981.html
>
> Given that the GHC patch is so small, I have a slight preference for
> keeping ld-wrapper unchanged and using the GHC patch.  To put it
> differently, the GHC patch is smaller and less error-prone than the
> changes that would need to be made in ld-wrapper.

I don't think that it is a good idea because any upstream change around
that code will break our package again and, going forward, we may find
other pieces of software making use of this gcc feature.  The patch is
small, but the effort to find it wasn't.

I like to fix things where the problem is, not working around it.

Fede




Information forwarded to bug-guix <at> gnu.org:
bug#25882; Package guix. (Tue, 07 Mar 2017 10:55:02 GMT) Full text and rfc822 format available.

Message #14 received at 25882 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Federico Beffa <beffa <at> ieee.org>
Cc: 25882 <at> debbugs.gnu.org
Subject: Re: bug#25882: gcc-wrapper doesn't handle response files
Date: Tue, 07 Mar 2017 11:53:57 +0100
[Message part 1 (text/plain, inline)]
Federico Beffa <beffa <at> ieee.org> skribis:

> ludo <at> gnu.org (Ludovic Courtès) writes:
>
>> Federico Beffa <beffa <at> fbengineering.ch> skribis:
>>
>>> gcc-wrapper doesn't handle compiler/linker flags passed through
>>> response files.
>>>
>>> One package which recently started using such files is GHC (I believe
>>> since 7.10.3).  For this reason we currently need to patch it.
>>> However, the problem is with our tool chain wrapper and not with GHC
>>> itself.
>>>
>>> See discussion at
>>> https://lists.gnu.org/archive/html/guix-devel/2017-01/msg01981.html
>>
>> Given that the GHC patch is so small, I have a slight preference for
>> keeping ld-wrapper unchanged and using the GHC patch.  To put it
>> differently, the GHC patch is smaller and less error-prone than the
>> changes that would need to be made in ld-wrapper.
>
> I don't think that it is a good idea because any upstream change around
> that code will break our package again and, going forward, we may find
> other pieces of software making use of this gcc feature.  The patch is
> small, but the effort to find it wasn't.
>
> I like to fix things where the problem is, not working around it.

On closer inspection, it’s an easy change to make.

Could you test the attached patch with GHC?

The way I would test it without rebuilding the world is by copying the
new ld-wrapper.in to ld-wrapper2.in (and thus keeping ld-wrapper.in
unchanged), and then adding it as an input to GHC (via
‘make-ld-wrapper’).  Commit 77db91addc57faa000db05563820f57a9ffdedfc
might serve as an example.

Thanks,
Ludo’.

[Message part 2 (text/x-patch, inline)]
diff --git a/gnu/packages/ld-wrapper.in b/gnu/packages/ld-wrapper.in
index ebfd8332c..ff086154a 100644
--- a/gnu/packages/ld-wrapper.in
+++ b/gnu/packages/ld-wrapper.in
@@ -15,7 +15,7 @@ main="(@ (gnu build-support ld-wrapper) ld-wrapper)"
 exec @GUILE@ -c "(load-compiled \"@SELF@.go\") (apply $main (cdr (command-line)))" "$@"
 !#
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2015, 2016 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo <at> gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -35,6 +35,7 @@ exec @GUILE@ -c "(load-compiled \"@SELF@.go\") (apply $main (cdr (command-line))
 (define-module (gnu build-support ld-wrapper)
   #:use-module (srfi srfi-1)
   #:use-module (ice-9 match)
+  #:autoload   (ice-9 rdelim) (read-string)
   #:export (ld-wrapper))
 
 ;;; Commentary:
@@ -222,9 +223,28 @@ impure library ~s~%"
               '()
               library-files))
 
+(define (expand-arguments args)
+  ;; Expand ARGS such that "response file" arguments, such as "@args.txt", are
+  ;; expanded.  See 'expandargv' in libiberty.
+  (define (response-file-arguments file)
+    (when %debug?
+      (format (current-error-port)
+              "ld-wrapper: reading arguments from '~a'~%" file))
+    (string-tokenize (call-with-input-file file read-string)))
+
+  (fold-right (lambda (arg result)
+                (if (string-prefix? "@" arg)
+                    (let ((file (string-drop arg 1)))
+                      (append (response-file-arguments file)
+                              result))
+                    (cons arg result)))
+              '()
+              args))
+
 (define (ld-wrapper . args)
   ;; Invoke the real `ld' with ARGS, augmented with `-rpath' switches.
-  (let* ((path (library-search-path args))
+  (let* ((args (expand-arguments args))
+         (path (library-search-path args))
          (libs (library-files-linked args path))
          (args (append args (rpath-arguments libs))))
     (when %debug?

Information forwarded to bug-guix <at> gnu.org:
bug#25882; Package guix. (Wed, 08 Mar 2017 12:58:02 GMT) Full text and rfc822 format available.

Message #17 received at 25882 <at> debbugs.gnu.org (full text, mbox):

From: Federico Beffa <beffa <at> ieee.org>
To: ludo <at> gnu.org (Ludovic Courtès)
Cc: 25882 <at> debbugs.gnu.org
Subject: Re: bug#25882: gcc-wrapper doesn't handle response files
Date: Wed, 08 Mar 2017 13:57:10 +0100
ludo <at> gnu.org (Ludovic Courtès) writes:

> On closer inspection, it’s an easy change to make.
>
> Could you test the attached patch with GHC?

Thanks for the patch.  I've tested it with GHC 8.0.2 and seems to work
for it.  However, this is an excerpt of the description of the use of
response files from the GCC manual:

--8<---------------cut here---------------start------------->8---
'@FILE'
     Read command-line options from FILE.  The options read are inserted
     in place of the original @FILE option.  If FILE does not exist, or
     cannot be read, then the option will be treated literally, and not
     removed.

     Options in FILE are separated by whitespace.  A whitespace
     character may be included in an option by surrounding the entire
     option in either single or double quotes.  Any character (including
     a backslash) may be included by prefixing the character to be
     included with a backslash.  The FILE may itself contain additional
     @FILE options; any such options will be processed recursively.
--8<---------------cut here---------------end--------------->8---

The patch doesn't seems to handle several things, such as missing files,
recursion and use of quotes.  I would suggest to try to match the
expected behavior in its entirety (or at least a larger part thereof).

Fede




Information forwarded to bug-guix <at> gnu.org:
bug#25882; Package guix. (Sat, 11 Mar 2017 13:48:01 GMT) Full text and rfc822 format available.

Message #20 received at 25882 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Federico Beffa <beffa <at> ieee.org>
Cc: 25882 <at> debbugs.gnu.org
Subject: Re: bug#25882: gcc-wrapper doesn't handle response files
Date: Sat, 11 Mar 2017 14:47:38 +0100
[Message part 1 (text/plain, inline)]
Federico Beffa <beffa <at> ieee.org> skribis:

> ludo <at> gnu.org (Ludovic Courtès) writes:
>
>> On closer inspection, it’s an easy change to make.
>>
>> Could you test the attached patch with GHC?
>
> Thanks for the patch.  I've tested it with GHC 8.0.2 and seems to work
> for it.  However, this is an excerpt of the description of the use of
> response files from the GCC manual:
>
> '@FILE'
>      Read command-line options from FILE.  The options read are inserted
>      in place of the original @FILE option.  If FILE does not exist, or
>      cannot be read, then the option will be treated literally, and not
>      removed.
>
>      Options in FILE are separated by whitespace.  A whitespace
>      character may be included in an option by surrounding the entire
>      option in either single or double quotes.  Any character (including
>      a backslash) may be included by prefixing the character to be
>      included with a backslash.  The FILE may itself contain additional
>      @FILE options; any such options will be processed recursively.

Oh, nice, I hadn’t seen this doc.

The attached version adds handling of unreadable files and recursion.
It does not address parsing of quote-delimited options though, but I’m
tempted to punt on that one.

Thoughts?

Thanks,
Ludo’.

[Message part 2 (text/x-patch, inline)]
diff --git a/gnu/packages/ld-wrapper.in b/gnu/packages/ld-wrapper.in
index ebfd8332c..82bd2196c 100644
--- a/gnu/packages/ld-wrapper.in
+++ b/gnu/packages/ld-wrapper.in
@@ -15,7 +15,7 @@ main="(@ (gnu build-support ld-wrapper) ld-wrapper)"
 exec @GUILE@ -c "(load-compiled \"@SELF@.go\") (apply $main (cdr (command-line)))" "$@"
 !#
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2015, 2016 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo <at> gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -35,6 +35,7 @@ exec @GUILE@ -c "(load-compiled \"@SELF@.go\") (apply $main (cdr (command-line))
 (define-module (gnu build-support ld-wrapper)
   #:use-module (srfi srfi-1)
   #:use-module (ice-9 match)
+  #:autoload   (ice-9 rdelim) (read-string)
   #:export (ld-wrapper))
 
 ;;; Commentary:
@@ -222,9 +223,44 @@ impure library ~s~%"
               '()
               library-files))
 
+(define (expand-arguments args)
+  ;; Expand ARGS such that "response file" arguments, such as "@args.txt", are
+  ;; expanded (info "(gcc) Overall Options").
+  (define (response-file-arguments file)
+    (when %debug?
+      (format (current-error-port)
+              "ld-wrapper: attempting to read arguments from '~a'~%" file))
+
+    ;; FIXME: Options can contain whitespace if they are protected by single
+    ;; or double quotes; this is not implemented here.
+    (string-tokenize (call-with-input-file file read-string)))
+
+  (define result
+    (fold-right (lambda (arg result)
+                  (if (string-prefix? "@" arg)
+                      (let ((file (string-drop arg 1)))
+                        (append (catch 'system-error
+                                  (lambda ()
+                                    (response-file-arguments file))
+                                  (lambda args
+                                    ;; FILE doesn't exist or cannot be read so
+                                    ;; leave ARG as is.
+                                    (list arg)))
+                                result))
+                      (cons arg result)))
+                '()
+                args))
+
+  ;; If there are "@" arguments in RESULT *and* we can expand them (they don't
+  ;; refer to nonexistent files), then recurse.
+  (if (equal? result args)
+      result
+      (expand-arguments result)))
+
 (define (ld-wrapper . args)
   ;; Invoke the real `ld' with ARGS, augmented with `-rpath' switches.
-  (let* ((path (library-search-path args))
+  (let* ((args (expand-arguments args))
+         (path (library-search-path args))
          (libs (library-files-linked args path))
          (args (append args (rpath-arguments libs))))
     (when %debug?

Changed bug title to 'ld-wrapper does not handle response files' from 'gcc-wrapper doesn't handle response files' Request was from ludo <at> gnu.org (Ludovic Courtès) to control <at> debbugs.gnu.org. (Sat, 11 Mar 2017 13:50:02 GMT) Full text and rfc822 format available.

Added tag(s) patch. Request was from ludo <at> gnu.org (Ludovic Courtès) to control <at> debbugs.gnu.org. (Sat, 11 Mar 2017 13:50:03 GMT) Full text and rfc822 format available.

Information forwarded to bug-guix <at> gnu.org:
bug#25882; Package guix. (Sun, 12 Mar 2017 11:37:01 GMT) Full text and rfc822 format available.

Message #27 received at 25882 <at> debbugs.gnu.org (full text, mbox):

From: Federico Beffa <beffa <at> ieee.org>
To: ludo <at> gnu.org (Ludovic Courtès)
Cc: 25882 <at> debbugs.gnu.org
Subject: Re: bug#25882: gcc-wrapper doesn't handle response files
Date: Sun, 12 Mar 2017 12:36:42 +0100
ludo <at> gnu.org (Ludovic Courtès) writes:

> The attached version adds handling of unreadable files and recursion.
> It does not address parsing of quote-delimited options though, but I’m
> tempted to punt on that one.

My worry is that not handling them may lead to hard to debug, guix
specific errors.

Fede




Reply sent to ludo <at> gnu.org (Ludovic Courtès):
You have taken responsibility. (Thu, 25 May 2017 12:43:02 GMT) Full text and rfc822 format available.

Notification sent to Federico Beffa <beffa <at> fbengineering.ch>:
bug acknowledged by developer. (Thu, 25 May 2017 12:43:02 GMT) Full text and rfc822 format available.

Message #32 received at 25882-done <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Federico Beffa <beffa <at> ieee.org>
Cc: 25882-done <at> debbugs.gnu.org
Subject: Re: bug#25882: gcc-wrapper doesn't handle response files
Date: Thu, 25 May 2017 14:42:32 +0200
Hi Federico,

Federico Beffa <beffa <at> ieee.org> skribis:

> ludo <at> gnu.org (Ludovic Courtès) writes:
>
>> The attached version adds handling of unreadable files and recursion.
>> It does not address parsing of quote-delimited options though, but I’m
>> tempted to punt on that one.
>
> My worry is that not handling them may lead to hard to debug, guix
> specific errors.

Since we’re about to freeze ‘core-updates’ and I’d like to make progress
on this front, I pushed the ld-wrapper change as commit
696487d665a616dfdd09272a7bff0bea0e19375d.

It’s not perfect as discussed earlier, but it’s known to be “good
enough” with GHC and Chromium, which are the only users of this I know
of.

Thanks,
Ludo’.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Fri, 23 Jun 2017 11:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 6 years and 280 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.