[PATCH] mips: use $dir for path to ldd-rewrite.sed

[PATCH] mips: use $dir for path to ldd-rewrite.sed

[Mike Frysinger]

The $dest variable is based on the top build dir but we need the ldd
rewrite variable to be based on the top source dir, so use $dir.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>

2012-04-16  Mike Frysinger  <vapier@gentoo.org>

	* sysdeps/unix/sysv/linux/mips/mips64/configure.in
	(ldd_rewrite_script): Change $dest to $dir.
	* sysdeps/unix/sysv/linux/mips/mips64/configure: Regenerate.
---
 sysdeps/unix/sysv/linux/mips/mips64/configure    |    2 +-
 sysdeps/unix/sysv/linux/mips/mips64/configure.in |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/mips/mips64/configure b/sysdeps/unix/sysv/linux/mips/mips64/configure
index c331f98..f86f327 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/configure
+++ b/sysdeps/unix/sysv/linux/mips/mips64/configure
@@ -1,4 +1,4 @@
 # This file is generated from configure.in by Autoconf.  DO NOT EDIT!
  # Local configure fragment for sysdeps/unix/sysv/linux/mips/mips64.
 
-ldd_rewrite_script=$dest/ldd-rewrite.sed
+ldd_rewrite_script=$dir/ldd-rewrite.sed
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/configure.in b/sysdeps/unix/sysv/linux/mips/mips64/configure.in
index a28638a..c137ec5 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/configure.in
+++ b/sysdeps/unix/sysv/linux/mips/mips64/configure.in
@@ -2,4 +2,4 @@ sinclude(./aclocal.m4)dnl Autoconf lossage
 GLIBC_PROVIDES dnl See aclocal.m4 in the top level source directory.
 # Local configure fragment for sysdeps/unix/sysv/linux/mips/mips64.
 
-ldd_rewrite_script=$dest/ldd-rewrite.sed
+ldd_rewrite_script=$dir/ldd-rewrite.sed
-- 
1.7.8.5

Re: [PATCH] mips: use $dir for path to ldd-rewrite.sed

[Carlos O'Donell]

On Wed, Apr 18, 2012 at 9:51 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> The $dest variable is based on the top build dir but we need the ldd
> rewrite variable to be based on the top source dir, so use $dir.
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>
> 2012-04-16  Mike Frysinger  <vapier@gentoo.org>
>
>        * sysdeps/unix/sysv/linux/mips/mips64/configure.in
>        (ldd_rewrite_script): Change $dest to $dir.
>        * sysdeps/unix/sysv/linux/mips/mips64/configure: Regenerate.

What problem are you trying to solve? What's the use case that's
broken? I remember seeing some patches fly by about $dest vs. $dir,
but I haven't fully absorbed the change.

Cheers,
Carlos.

Re: [PATCH] mips: use $dir for path to ldd-rewrite.sed

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 855 bytes --]

On Wednesday 18 April 2012 22:22:45 Carlos O'Donell wrote:
> On Wed, Apr 18, 2012 at 9:51 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> > The $dest variable is based on the top build dir but we need the ldd
> > rewrite variable to be based on the top source dir, so use $dir.
> > 
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> > 
> > 2012-04-16  Mike Frysinger  <vapier@gentoo.org>
> > 
> >        * sysdeps/unix/sysv/linux/mips/mips64/configure.in
> >        (ldd_rewrite_script): Change $dest to $dir.
> >        * sysdeps/unix/sysv/linux/mips/mips64/configure: Regenerate.
> 
> What problem are you trying to solve? What's the use case that's
> broken? I remember seeing some patches fly by about $dest vs. $dir,
> but I haven't fully absorbed the change.

http://sourceware.org/ml/libc-alpha/2012-04/msg00588.html
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mips: use $dir for path to ldd-rewrite.sed

[Carlos O'Donell]

On Wed, Apr 18, 2012 at 10:28 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Wednesday 18 April 2012 22:22:45 Carlos O'Donell wrote:
>> On Wed, Apr 18, 2012 at 9:51 PM, Mike Frysinger <vapier@gentoo.org> wrote:
>> > The $dest variable is based on the top build dir but we need the ldd
>> > rewrite variable to be based on the top source dir, so use $dir.
>> >
>> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> >
>> > 2012-04-16  Mike Frysinger  <vapier@gentoo.org>
>> >
>> >        * sysdeps/unix/sysv/linux/mips/mips64/configure.in
>> >        (ldd_rewrite_script): Change $dest to $dir.
>> >        * sysdeps/unix/sysv/linux/mips/mips64/configure: Regenerate.
>>
>> What problem are you trying to solve? What's the use case that's
>> broken? I remember seeing some patches fly by about $dest vs. $dir,
>> but I haven't fully absorbed the change.
>
> http://sourceware.org/ml/libc-alpha/2012-04/msg00588.html

Thank you, that makes sense.

The patch looks good from that perspective.

I'd say check this on Friday if nobody objects.

Cheers,
Carlos.

Re: [PATCH] mips: use $dir for path to ldd-rewrite.sed

[Joseph S. Myers]

On Wed, 18 Apr 2012, Mike Frysinger wrote:

> The $dest variable is based on the top build dir but we need the ldd
> rewrite variable to be based on the top source dir, so use $dir.

Could you explain:

* what the semantics of $dest are, and where it is set;

* what the semantics of $dir are, and where it is set;

* what the semantics of $ldd_rewrite_script are (i.e. how the value is 
used, which determines what sort of path is appropriate);

* how this patch has been tested (what configurations, using relative or 
absolute paths with however many directory levels to whichever are 
relevant of the main source directory and the ports add-on)?

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] mips: use $dir for path to ldd-rewrite.sed

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 356 bytes --]

On Thursday 19 April 2012 15:51:40 Joseph S. Myers wrote:
> On Wed, 18 Apr 2012, Mike Frysinger wrote:
> > The $dest variable is based on the top build dir but we need the ldd
> > rewrite variable to be based on the top source dir, so use $dir.
> 
> Could you explain:

please see the URL i posted to carlos that has the background research
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mips: use $dir for path to ldd-rewrite.sed

[Joseph S. Myers]

On Thu, 19 Apr 2012, Mike Frysinger wrote:

> On Thursday 19 April 2012 15:51:40 Joseph S. Myers wrote:
> > On Wed, 18 Apr 2012, Mike Frysinger wrote:
> > > The $dest variable is based on the top build dir but we need the ldd
> > > rewrite variable to be based on the top source dir, so use $dir.
> > 
> > Could you explain:
> 
> please see the URL i posted to carlos that has the background research

That certainly doesn't say how you tested the patch (presumably in at 
least one case that failed before the patch and passed afterwards, but you 
don't identify what cases).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] mips: use $dir for path to ldd-rewrite.sed

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1004 bytes --]

On Thursday 19 April 2012 16:20:39 Joseph S. Myers wrote:
> On Thu, 19 Apr 2012, Mike Frysinger wrote:
> > On Thursday 19 April 2012 15:51:40 Joseph S. Myers wrote:
> > > On Wed, 18 Apr 2012, Mike Frysinger wrote:
> > > > The $dest variable is based on the top build dir but we need the ldd
> > > > rewrite variable to be based on the top source dir, so use $dir.
> > > 
> > > Could you explain:
> > please see the URL i posted to carlos that has the background research
> 
> That certainly doesn't say how you tested the patch (presumably in at
> least one case that failed before the patch and passed afterwards, but you
> don't identify what cases).

i showed how it fails today:
	http://sourceware.org/ml/libc-alpha/2012-04/msg00591.html

so after i made my fix, i did the same thing and saw the correct path.  i 
didn't build test mips because ia64 was affected by the same issue and fixing it 
there made things build fine.  ergo making the same fix to mips should work.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mips: use $dir for path to ldd-rewrite.sed

[Joseph S. Myers]

On Thu, 19 Apr 2012, Mike Frysinger wrote:

> On Thursday 19 April 2012 16:20:39 Joseph S. Myers wrote:
> > On Thu, 19 Apr 2012, Mike Frysinger wrote:
> > > On Thursday 19 April 2012 15:51:40 Joseph S. Myers wrote:
> > > > On Wed, 18 Apr 2012, Mike Frysinger wrote:
> > > > > The $dest variable is based on the top build dir but we need the ldd
> > > > > rewrite variable to be based on the top source dir, so use $dir.
> > > > 
> > > > Could you explain:
> > > please see the URL i posted to carlos that has the background research
> > 
> > That certainly doesn't say how you tested the patch (presumably in at
> > least one case that failed before the patch and passed afterwards, but you
> > don't identify what cases).
> 
> i showed how it fails today:
> 	http://sourceware.org/ml/libc-alpha/2012-04/msg00591.html
> 
> so after i made my fix, i did the same thing and saw the correct path.  i 
> didn't build test mips because ia64 was affected by the same issue and fixing it 
> there made things build fine.  ergo making the same fix to mips should work.

I still don't see a good explanation of what all these variables are, 
along the following lines:

* $dir is a directory that may be an absolute path to a directory in an 
add-on, if configured using an absolute path to the add-on or a relative 
path to the add-on but an absolute path to srcdir, or a relative path to 
that directory from the srcdir if both configure-time paths were relative.

* $dest is a path to $dir that is always valid from the top of the build 
directory (and is relative if both the source directory and the path to 
any add-on were relative, absolute if either of those was absolute).

* $ldd_rewrite_script may be either absolute or relative to the toplevel 
source directory, but not relative to the build directory.  It is used in 
the elf/ subdirectory of the source directory via $(patsubst 
$(..)/%,/%,$(..)$(ldd-rewrite-script)), which means "use 
$(ldd-rewrite-script) if it's an absolute path, use 
../$(ldd-rewrite-script) if it's a relative path".

Is that the correct meaning of the three variables?  If so, the patch is 
OK, but you need to include such explanations in self-contained patch 
submissions to make it clear to readers why the patch is correct without 
requiring multiple messages from other threads to be followed.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] mips: use $dir for path to ldd-rewrite.sed

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 509 bytes --]

On Thursday 19 April 2012 17:38:26 Joseph S. Myers wrote:
> Is that the correct meaning of the three variables?

yes, as the referenced thread more or less said

> If so, the patch is
> OK, but you need to include such explanations in self-contained patch
> submissions to make it clear to readers why the patch is correct without
> requiring multiple messages from other threads to be followed.

the documentation on these variables is, as i pointed out in the other thread, 
non-existent
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mips: use $dir for path to ldd-rewrite.sed

[Joseph S. Myers]

On Thu, 19 Apr 2012, Mike Frysinger wrote:

> > If so, the patch is
> > OK, but you need to include such explanations in self-contained patch
> > submissions to make it clear to readers why the patch is correct without
> > requiring multiple messages from other threads to be followed.
> 
> the documentation on these variables is, as i pointed out in the other thread, 
> non-existent

Such documentation being nonexistent in the source tree is all the more 
reason why a patch submission involving these variables needs to include 
an explanation directly in the submission message to make things easy for 
readers of the patch submission.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] mips: use $dir for path to ldd-rewrite.sed

[Carlos O'Donell]

On Thu, Apr 19, 2012 at 5:38 PM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> On Thu, 19 Apr 2012, Mike Frysinger wrote:
>
>> On Thursday 19 April 2012 16:20:39 Joseph S. Myers wrote:
>> > On Thu, 19 Apr 2012, Mike Frysinger wrote:
>> > > On Thursday 19 April 2012 15:51:40 Joseph S. Myers wrote:
>> > > > On Wed, 18 Apr 2012, Mike Frysinger wrote:
>> > > > > The $dest variable is based on the top build dir but we need the ldd
>> > > > > rewrite variable to be based on the top source dir, so use $dir.
>> > > >
>> > > > Could you explain:
>> > > please see the URL i posted to carlos that has the background research
>> >
>> > That certainly doesn't say how you tested the patch (presumably in at
>> > least one case that failed before the patch and passed afterwards, but you
>> > don't identify what cases).
>>
>> i showed how it fails today:
>>       http://sourceware.org/ml/libc-alpha/2012-04/msg00591.html
>>
>> so after i made my fix, i did the same thing and saw the correct path.  i
>> didn't build test mips because ia64 was affected by the same issue and fixing it
>> there made things build fine.  ergo making the same fix to mips should work.
>
> I still don't see a good explanation of what all these variables are,
> along the following lines:
>
> * $dir is a directory that may be an absolute path to a directory in an
> add-on, if configured using an absolute path to the add-on or a relative
> path to the add-on but an absolute path to srcdir, or a relative path to
> that directory from the srcdir if both configure-time paths were relative.
>
> * $dest is a path to $dir that is always valid from the top of the build
> directory (and is relative if both the source directory and the path to
> any add-on were relative, absolute if either of those was absolute).
>
> * $ldd_rewrite_script may be either absolute or relative to the toplevel
> source directory, but not relative to the build directory.  It is used in
> the elf/ subdirectory of the source directory via $(patsubst
> $(..)/%,/%,$(..)$(ldd-rewrite-script)), which means "use
> $(ldd-rewrite-script) if it's an absolute path, use
> ../$(ldd-rewrite-script) if it's a relative path".
>
> Is that the correct meaning of the three variables?  If so, the patch is
> OK, but you need to include such explanations in self-contained patch
> submissions to make it clear to readers why the patch is correct without
> requiring multiple messages from other threads to be followed.

Mike, Joseph,

Given that these definitions are correct could someone please add an
internals page to the wiki describing them?

I assume that once this page is created that the patch should go in to
fix MIPS and IA64.

Cheers,
Carlos.

Re: [PATCH] mips: use $dir for path to ldd-rewrite.sed

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 427 bytes --]

On Sunday 22 April 2012 11:45:42 Carlos O'Donell wrote:
> Given that these definitions are correct could someone please add an
> internals page to the wiki describing them?

where would you suggest ?  i don't see any wiki page talking about ports or 
addons in general.

> I assume that once this page is created that the patch should go in to
> fix MIPS and IA64.

ia64 will start off with the correct value
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mips: use $dir for path to ldd-rewrite.sed

[Carlos O'Donell]

On Tue, Apr 24, 2012 at 12:02 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Sunday 22 April 2012 11:45:42 Carlos O'Donell wrote:
>> Given that these definitions are correct could someone please add an
>> internals page to the wiki describing them?
>
> where would you suggest ?  i don't see any wiki page talking about ports or
> addons in general.

You're breaking new ground.

I suggest a brand new page called "Build Framework" under the
Development->Internals Documentation section.
e.g.
http://sourceware.org/glibc/wiki/#InternalsDocumentation

Add your description there about this variable there and how it's used.

>> I assume that once this page is created that the patch should go in to
>> fix MIPS and IA64.
>
> ia64 will start off with the correct value
> -mike

Re: [PATCH] mips: use $dir for path to ldd-rewrite.sed

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 334 bytes --]

On Wednesday 18 April 2012 21:51:32 Mike Frysinger wrote:
> The $dest variable is based on the top build dir but we need the ldd
> rewrite variable to be based on the top source dir, so use $dir.

at this point, i don't know what the status of this patch is, so i'm just 
going to let the mips maintainer merge/whatever.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mips: use $dir for path to ldd-rewrite.sed

[Joseph S. Myers]

On Sun, 22 Apr 2012, Mike Frysinger wrote:

> On Wednesday 18 April 2012 21:51:32 Mike Frysinger wrote:
> > The $dest variable is based on the top build dir but we need the ldd
> > rewrite variable to be based on the top source dir, so use $dir.
> 
> at this point, i don't know what the status of this patch is, so i'm just 
> going to let the mips maintainer merge/whatever.

I already said it was OK if my analysis of the variables involved was 
correct - and, separately, that the patch submission ought to have 
contained more information (which is a point to take account of in future 
patch submissions for other patches).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] mips: use $dir for path to ldd-rewrite.sed

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 581 bytes --]

On Monday 23 April 2012 05:43:01 Joseph S. Myers wrote:
> On Sun, 22 Apr 2012, Mike Frysinger wrote:
> > On Wednesday 18 April 2012 21:51:32 Mike Frysinger wrote:
> > > The $dest variable is based on the top build dir but we need the ldd
> > > rewrite variable to be based on the top source dir, so use $dir.
> > 
> > at this point, i don't know what the status of this patch is, so i'm just
> > going to let the mips maintainer merge/whatever.
> 
> I already said it was OK if my analysis of the variables involved was
> correct

i've pushed it then.  thanks.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]