diff mbox series

[v4] ld: Support LD_UNDER_TEST environment variable

Message ID 20240317121524.799242-1-hjl.tools@gmail.com
State Committed
Headers
Series [v4] ld: Support LD_UNDER_TEST environment variable |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Testing passed

Commit Message

H.J. Lu March 17, 2024, 12:15 p.m. UTC 
  Support LD_UNDER_TEST environment variable to test a different linker.
Issue an error if LD_UNDER_TEST isn't an absolute full path.

	* testsuite/config/default.exp: If LD_UNDER_TEST environment
	variable exists, set ld and LD to it and set up tmpdir/ld/ld.
	Issue an error if LD_UNDER_TEST isn't an absolute full path.
---
 ld/testsuite/config/default.exp | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

Comments

Nick Clifton March 19, 2024, 12:06 p.m. UTC | #1 
Hi H.J.

> Support LD_UNDER_TEST environment variable to test a different linker.
> Issue an error if LD_UNDER_TEST isn't an absolute full path.

I would ask for one more change:

> +	perror "**************************************************"
> +	perror "$env(LD_UNDER_TEST) isn't an absolute full path."
> +	perror "**************************************************"

If I am reading this correctly the error message will show the contents
of the LD_UNDER_TEST environment variable, but not the name of the variable.
This could be confusing if the user does not realise that they have the
variable set.  Therefore please could you update the message to something
like:

   perror "Environment variable LD_UNDER_TEST ($env(LD_UNDER_TEST)) is not an absolute full path"

And similarly for the other error message.

Patch approved with these changes,.

Cheers
   Nick
H.J. Lu March 19, 2024, 2:07 p.m. UTC | #2 
On Tue, Mar 19, 2024 at 5:07 AM Nick Clifton <nickc@redhat.com> wrote:
>
> Hi H.J.
>
> > Support LD_UNDER_TEST environment variable to test a different linker.
> > Issue an error if LD_UNDER_TEST isn't an absolute full path.
>
> I would ask for one more change:
>
> > +     perror "**************************************************"
> > +     perror "$env(LD_UNDER_TEST) isn't an absolute full path."
> > +     perror "**************************************************"
>
> If I am reading this correctly the error message will show the contents
> of the LD_UNDER_TEST environment variable, but not the name of the variable.
> This could be confusing if the user does not realise that they have the
> variable set.  Therefore please could you update the message to something
> like:
>
>    perror "Environment variable LD_UNDER_TEST ($env(LD_UNDER_TEST)) is not an absolute full path"
>
> And similarly for the other error message.

Fixed.

> Patch approved with these changes,.
>
> Cheers
>    Nick
>

This is what I am checking in.

Thanks.
diff mbox series

Patch

diff --git a/ld/testsuite/config/default.exp b/ld/testsuite/config/default.exp
index 705543054c2..ed6002e6427 100644
--- a/ld/testsuite/config/default.exp
+++ b/ld/testsuite/config/default.exp
@@ -21,6 +21,23 @@ 
 # Written by Jeffrey Wheat (cassidy@cygnus.com)
 #
 
+if [info exists env(LD_UNDER_TEST)] {
+    # LD_UNDER_TEST must be an absolute full path.
+    if {[file pathtype $env(LD_UNDER_TEST)] ne "absolute"} {
+	perror "**************************************************"
+	perror "$env(LD_UNDER_TEST) isn't an absolute full path."
+	perror "**************************************************"
+	exit 1
+    } elseif {![file exists $env(LD_UNDER_TEST)]} {
+	perror "**************************************************"
+	perror "$env(LD_UNDER_TEST) doesn't exist."
+	perror "**************************************************"
+	exit 1
+    }
+    set ld "$env(LD_UNDER_TEST)"
+    set LD "$ld"
+}
+
 if ![info exists ld] then {
     set ld [findfile $base_dir/ld-new $base_dir/ld-new [transform ld]]
 }
@@ -64,12 +81,17 @@  remote_exec host "mkdir -p tmpdir"
 if {[info exists ld_testsuite_bindir]} {
     set gcc_B_opt "-B$ld_testsuite_bindir/"
 } else {
-    if {![file isdirectory tmpdir/ld]} then {
-	catch "exec mkdir tmpdir/ld" status
+    # Delete tmpdir/ld first to remove tmpdir/ld/ld created by the
+    # previous LD_UNDER_TEST runs.
+    file delete -force tmpdir/ld
+    catch "exec mkdir tmpdir/ld" status
+    if [info exists env(LD_UNDER_TEST)] {
+	catch "exec ln -s $env(LD_UNDER_TEST) tmpdir/ld/ld" status
+    } else {
 	catch "exec ln -s ../../ld-new tmpdir/ld/ld" status
-	catch "exec ln -s ld tmpdir/ld/collect-ld" status
-	catch "exec ln -s ../../../gas/as-new tmpdir/ld/as" status
     }
+    catch "exec ln -s ld tmpdir/ld/collect-ld" status
+    catch "exec ln -s ../../../gas/as-new tmpdir/ld/as" status
     set gcc_B_opt "-B[pwd]/tmpdir/ld/"
 }