test 104 fails on windows: missing mkstemps

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

test 104 fails on windows: missing mkstemps

avih
Test 104 log on windows (both tcc32 and tcc 64):

Test: 104_inline_test...
--- 104_inline_test.expect      2019-06-17 23:42:00.162697100 +0300
+++ 104_inline_test.output      2019-06-17 23:42:35.531550400 +0300
@@ -1,25 +1,2 @@
-extern_extern_postdeclared
-extern_extern_postdeclared2
-extern_extern_predeclared
-extern_extern_predeclared2
-extern_extern_prepostdeclared
-extern_extern_prepostdeclared2
-extern_extern_undeclared
-extern_extern_undeclared2
-extern_postdeclared
-extern_postdeclared2
-extern_predeclared
-extern_predeclared2
-extern_prepostdeclared
-extern_undeclared
-extern_undeclared2
-inst2_extern_inline_postdeclared
-inst2_extern_inline_predeclared
-inst3_extern_inline_predeclared
-inst_extern_inline_postdeclared
-inst_extern_inline_predeclared
-main
-noinst_extern_inline_func
-noinst_extern_inline_postdeclared
-noinst_extern_inline_postdeclared2
-noinst_extern_inline_undeclared
+104_inline_test.c:30: warning: implicit declaration of function 'mkstemps'
+tcc: error: undefined symbol 'mkstemps'
make[1]: *** [Makefile:70: 104_inline_test.test] Error 1
Test: 105_local_extern...
make[1]: Target 'all' not remade because of errors.



_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

Christian Jullien-3

Yes it has been previously reported.

 

Michael, as said in a private mail, I agree with you that this test should be skipped on Windows.

 

C.

 

From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=[hidden email]] On Behalf Of avih
Sent: Monday, June 17, 2019 22:46
To: Tinycc-devel Mailing List
Subject: [Tinycc-devel] test 104 fails on windows: missing mkstemps

 

Test 104 log on windows (both tcc32 and tcc 64):

 

Test: 104_inline_test...
--- 104_inline_test.expect      2019-06-17 23:42:00.162697100 +0300
+++ 104_inline_test.output      2019-06-17 23:42:35.531550400 +0300
@@ -1,25 +1,2 @@
-extern_extern_postdeclared
-extern_extern_postdeclared2
-extern_extern_predeclared
-extern_extern_predeclared2
-extern_extern_prepostdeclared
-extern_extern_prepostdeclared2
-extern_extern_undeclared
-extern_extern_undeclared2
-extern_postdeclared
-extern_postdeclared2
-extern_predeclared
-extern_predeclared2
-extern_prepostdeclared
-extern_undeclared
-extern_undeclared2
-inst2_extern_inline_postdeclared
-inst2_extern_inline_predeclared
-inst3_extern_inline_predeclared
-inst_extern_inline_postdeclared
-inst_extern_inline_predeclared
-main
-noinst_extern_inline_func
-noinst_extern_inline_postdeclared
-noinst_extern_inline_postdeclared2
-noinst_extern_inline_undeclared
+104_inline_test.c:30: warning: implicit declaration of function 'mkstemps'
+tcc: error: undefined symbol 'mkstemps'
make[1]: *** [Makefile:70: 104_inline_test.test] Error 1
Test: 105_local_extern...
make[1]: Target 'all' not remade because of errors.

 


_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

avih
Wouldn't it be better to just create a known/fixed file instead? (assuming the test doesn't need explicitly mkstemps, I didn't look at its actual code).


On Monday, June 17, 2019 11:50 PM, Christian Jullien <[hidden email]> wrote:


Yes it has been previously reported.
 
Michael, as said in a private mail, I agree with you that this test should be skipped on Windows.
 
C.
 
From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=[hidden email]] On Behalf Of avih
Sent: Monday, June 17, 2019 22:46
To: Tinycc-devel Mailing List
Subject: [Tinycc-devel] test 104 fails on windows: missing mkstemps
 
Test 104 log on windows (both tcc32 and tcc 64):
 
Test: 104_inline_test...
--- 104_inline_test.expect      2019-06-17 23:42:00.162697100 +0300
+++ 104_inline_test.output      2019-06-17 23:42:35.531550400 +0300
@@ -1,25 +1,2 @@
-extern_extern_postdeclared
-extern_extern_postdeclared2
-extern_extern_predeclared
-extern_extern_predeclared2
-extern_extern_prepostdeclared
-extern_extern_prepostdeclared2
-extern_extern_undeclared
-extern_extern_undeclared2
-extern_postdeclared
-extern_postdeclared2
-extern_predeclared
-extern_predeclared2
-extern_prepostdeclared
-extern_undeclared
-extern_undeclared2
-inst2_extern_inline_postdeclared
-inst2_extern_inline_predeclared
-inst3_extern_inline_predeclared
-inst_extern_inline_postdeclared
-inst_extern_inline_predeclared
-main
-noinst_extern_inline_func
-noinst_extern_inline_postdeclared
-noinst_extern_inline_postdeclared2
-noinst_extern_inline_undeclared
+104_inline_test.c:30: warning: implicit declaration of function 'mkstemps'
+tcc: error: undefined symbol 'mkstemps'
make[1]: *** [Makefile:70: 104_inline_test.test] Error 1
Test: 105_local_extern...
make[1]: Target 'all' not remade because of errors.
 



_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

avih
Hmm.. I now see that test 104 uses signal and nm, so it might require some effort to make it work on windows.

Nevertheless, considering the recent breakage specifically on windows from related code, I think it would be beneficial if this test could be made to work on windows,


On Monday, June 17, 2019 11:54 PM, avih <[hidden email]> wrote:


Wouldn't it be better to just create a known/fixed file instead? (assuming the test doesn't need explicitly mkstemps, I didn't look at its actual code).


On Monday, June 17, 2019 11:50 PM, Christian Jullien <[hidden email]> wrote:


Yes it has been previously reported.
 
Michael, as said in a private mail, I agree with you that this test should be skipped on Windows.
 
C.
 
From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=[hidden email]] On Behalf Of avih
Sent: Monday, June 17, 2019 22:46
To: Tinycc-devel Mailing List
Subject: [Tinycc-devel] test 104 fails on windows: missing mkstemps
 
Test 104 log on windows (both tcc32 and tcc 64):
 
Test: 104_inline_test...
--- 104_inline_test.expect      2019-06-17 23:42:00.162697100 +0300
+++ 104_inline_test.output      2019-06-17 23:42:35.531550400 +0300
@@ -1,25 +1,2 @@
-extern_extern_postdeclared
-extern_extern_postdeclared2
-extern_extern_predeclared
-extern_extern_predeclared2
-extern_extern_prepostdeclared
-extern_extern_prepostdeclared2
-extern_extern_undeclared
-extern_extern_undeclared2
-extern_postdeclared
-extern_postdeclared2
-extern_predeclared
-extern_predeclared2
-extern_prepostdeclared
-extern_undeclared
-extern_undeclared2
-inst2_extern_inline_postdeclared
-inst2_extern_inline_predeclared
-inst3_extern_inline_predeclared
-inst_extern_inline_postdeclared
-inst_extern_inline_predeclared
-main
-noinst_extern_inline_func
-noinst_extern_inline_postdeclared
-noinst_extern_inline_postdeclared2
-noinst_extern_inline_undeclared
+104_inline_test.c:30: warning: implicit declaration of function 'mkstemps'
+tcc: error: undefined symbol 'mkstemps'
make[1]: *** [Makefile:70: 104_inline_test.test] Error 1
Test: 105_local_extern...
make[1]: Target 'all' not remade because of errors.
 





_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

avih
After commit d39c49db  Remove empty conditional _WIN32 code

and some hacking of the code (it's an unhealthy mix of basically running a shell script from a program compiled using tcc for windows), I get the following 2 diffs:

+inst_extern_inline_postdeclared
+inst_extern_inline_predeclared

and

-inst_extern_inline_postdeclared
-inst_extern_inline_predeclared

I'm running it in a cygwin environment and the tools (nm, sort, gawk) are cygwin tools, while the tested tcc is normal mingw tcc for windows (which I build in a reproducible way using my script).

Regardless of these two diffs, I think the test should be composed of a program and a normal shell script (which uses mktemp, awk, sort etc), so that the paths are consistent between the tools.

Also, the TCC path is hardcoded at the test, but in fact it's parametric at the makefile as $(TCC), so it's better to use that instead (but then there are forward/backward slash issues which need to be handled too, because system(...) in win32 expects backward slashes, but $(TCC) at the makefile has forward slashes). Making this a program + a script should implicitly solve this issue as well.

After all, a working shell+tools is assumed for this test anyway, but the current way of using system(...) from a win32 program (compiled using tcc for windows) invokes a windows shell which can be inconsistent with the actual shell where `make` runs.

Avi




On Tuesday, June 18, 2019 12:11 AM, avih <[hidden email]> wrote:


Hmm.. I now see that test 104 uses signal and nm, so it might require some effort to make it work on windows.

Nevertheless, considering the recent breakage specifically on windows from related code, I think it would be beneficial if this test could be made to work on windows,


On Monday, June 17, 2019 11:54 PM, avih <[hidden email]> wrote:


Wouldn't it be better to just create a known/fixed file instead? (assuming the test doesn't need explicitly mkstemps, I didn't look at its actual code).


On Monday, June 17, 2019 11:50 PM, Christian Jullien <[hidden email]> wrote:


Yes it has been previously reported.
 
Michael, as said in a private mail, I agree with you that this test should be skipped on Windows.
 
C.
 
From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=[hidden email]] On Behalf Of avih
Sent: Monday, June 17, 2019 22:46
To: Tinycc-devel Mailing List
Subject: [Tinycc-devel] test 104 fails on windows: missing mkstemps
 
Test 104 log on windows (both tcc32 and tcc 64):
 
Test: 104_inline_test...
--- 104_inline_test.expect      2019-06-17 23:42:00.162697100 +0300
+++ 104_inline_test.output      2019-06-17 23:42:35.531550400 +0300
@@ -1,25 +1,2 @@
-extern_extern_postdeclared
-extern_extern_postdeclared2
-extern_extern_predeclared
-extern_extern_predeclared2
-extern_extern_prepostdeclared
-extern_extern_prepostdeclared2
-extern_extern_undeclared
-extern_extern_undeclared2
-extern_postdeclared
-extern_postdeclared2
-extern_predeclared
-extern_predeclared2
-extern_prepostdeclared
-extern_undeclared
-extern_undeclared2
-inst2_extern_inline_postdeclared
-inst2_extern_inline_predeclared
-inst3_extern_inline_predeclared
-inst_extern_inline_postdeclared
-inst_extern_inline_predeclared
-main
-noinst_extern_inline_func
-noinst_extern_inline_postdeclared
-noinst_extern_inline_postdeclared2
-noinst_extern_inline_undeclared
+104_inline_test.c:30: warning: implicit declaration of function 'mkstemps'
+tcc: error: undefined symbol 'mkstemps'
make[1]: *** [Makefile:70: 104_inline_test.test] Error 1
Test: 105_local_extern...
make[1]: Target 'all' not remade because of errors.
 







_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

avih
Well, the diffs are not really diffs. They just moved. Looks like `sort` didn't work as expected, or maybe it's some locale issue (mine is en_US.UTF-8 at cygwin, and en-US at windows).

A script should handle this too, possibly with LC_ALL=C (and make sure the reference file is generated with the same sort locale).

If someone could split it to program+script then I can test the test in various use cases and make adjustment if required (I'm terrible with Makefiles but reasonably useful with shell).


On Tuesday, June 18, 2019 9:50 PM, avih <[hidden email]> wrote:


After commit d39c49db  Remove empty conditional _WIN32 code

and some hacking of the code (it's an unhealthy mix of basically running a shell script from a program compiled using tcc for windows), I get the following 2 diffs:

+inst_extern_inline_postdeclared
+inst_extern_inline_predeclared

and

-inst_extern_inline_postdeclared
-inst_extern_inline_predeclared

I'm running it in a cygwin environment and the tools (nm, sort, gawk) are cygwin tools, while the tested tcc is normal mingw tcc for windows (which I build in a reproducible way using my script).

Regardless of these two diffs, I think the test should be composed of a program and a normal shell script (which uses mktemp, awk, sort etc), so that the paths are consistent between the tools.

Also, the TCC path is hardcoded at the test, but in fact it's parametric at the makefile as $(TCC), so it's better to use that instead (but then there are forward/backward slash issues which need to be handled too, because system(...) in win32 expects backward slashes, but $(TCC) at the makefile has forward slashes). Making this a program + a script should implicitly solve this issue as well.

After all, a working shell+tools is assumed for this test anyway, but the current way of using system(...) from a win32 program (compiled using tcc for windows) invokes a windows shell which can be inconsistent with the actual shell where `make` runs.

Avi




On Tuesday, June 18, 2019 12:11 AM, avih <[hidden email]> wrote:


Hmm.. I now see that test 104 uses signal and nm, so it might require some effort to make it work on windows.

Nevertheless, considering the recent breakage specifically on windows from related code, I think it would be beneficial if this test could be made to work on windows,


On Monday, June 17, 2019 11:54 PM, avih <[hidden email]> wrote:


Wouldn't it be better to just create a known/fixed file instead? (assuming the test doesn't need explicitly mkstemps, I didn't look at its actual code).


On Monday, June 17, 2019 11:50 PM, Christian Jullien <[hidden email]> wrote:


Yes it has been previously reported.
 
Michael, as said in a private mail, I agree with you that this test should be skipped on Windows.
 
C.
 
From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=[hidden email]] On Behalf Of avih
Sent: Monday, June 17, 2019 22:46
To: Tinycc-devel Mailing List
Subject: [Tinycc-devel] test 104 fails on windows: missing mkstemps
 
Test 104 log on windows (both tcc32 and tcc 64):
 
Test: 104_inline_test...
--- 104_inline_test.expect      2019-06-17 23:42:00.162697100 +0300
+++ 104_inline_test.output      2019-06-17 23:42:35.531550400 +0300
@@ -1,25 +1,2 @@
-extern_extern_postdeclared
-extern_extern_postdeclared2
-extern_extern_predeclared
-extern_extern_predeclared2
-extern_extern_prepostdeclared
-extern_extern_prepostdeclared2
-extern_extern_undeclared
-extern_extern_undeclared2
-extern_postdeclared
-extern_postdeclared2
-extern_predeclared
-extern_predeclared2
-extern_prepostdeclared
-extern_undeclared
-extern_undeclared2
-inst2_extern_inline_postdeclared
-inst2_extern_inline_predeclared
-inst3_extern_inline_predeclared
-inst_extern_inline_postdeclared
-inst_extern_inline_predeclared
-main
-noinst_extern_inline_func
-noinst_extern_inline_postdeclared
-noinst_extern_inline_postdeclared2
-noinst_extern_inline_undeclared
+104_inline_test.c:30: warning: implicit declaration of function 'mkstemps'
+tcc: error: undefined symbol 'mkstemps'
make[1]: *** [Makefile:70: 104_inline_test.test] Error 1
Test: 105_local_extern...
make[1]: Target 'all' not remade because of errors.
 









_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

Christian Jullien-3

Avih,

I quickly hacked 104 test to start to make it work but I’m not the author of this test. I let Petr improve it.

I fully agree with you that mixing C code and calling system (which forks a cmd.exe on Windows) to finally execute gnu commands that require Cygwin is a BAD idea!!

As you said, it’s much easier if 104 test generates a .o which the whole ‘unix’ infrastructure test will check. I let the maintainer of this test decide what to do.

 

About the diff, I don’t understand quite well what happens. Among other, it calls system(“sort …”); which spawns cmd.exe and then execute first sort seen in path. Depending on your PATH it can be Windows sort (if Windows path comes first) or Cygwin version if this one comes first.

 

Can you please try this diff which works as good for me but now forces sort Windows version:

 

diff --git a/tests/tests2/104_inline_test.c b/tests/tests2/104_inline_test.c

index cb288d2..d191602 100644

--- a/tests/tests2/104_inline_test.c

+++ b/tests/tests2/104_inline_test.c

@@ -5,8 +5,8 @@

 

#if __linux__ || __APPLE__

#define SYS_WHICH_NM "which nm >/dev/null 2>&1"

+#define SYS_SORT     "sort"

#define TCC_COMPILER "../../tcc"

-#define SYS_AWK

 

char c[]="/tmp/tcc-XXXXXX"; char o[]="/tmp/tcc-XXXXXX";

static int mktempfile(char *buf)

@@ -15,6 +15,7 @@ static int mktempfile(char *buf)

}

#elif defined(_WIN32)

#define SYS_WHICH_NM  "which nm >nul 2>&1"

+#define SYS_SORT      "%systemroot%\\system32\\sort /LOCALE C"

 

#if defined(_WIN64)

#define TCC_COMPILER "..\\..\\win32\\x86_64-win32-tcc"

@@ -72,7 +73,7 @@ int main(int C, char **V)

         sprintf(buf, "%s -c -xc %s -o %s", V[1]?V[1]:TCC_COMPILER, c, o); if(0!=system(buf)){ r=1;goto out;}

         sprintf(buf, "nm -Ptx %s > %s", o, c); if(system(buf)) {r=1;goto out;}

         sprintf(buf, "gawk '{ if($2 == \"T\") print $1 }' %s > %s", c, o); if(system(buf)) {r=1;goto out;}

-        sprintf(buf, "sort %s", o); if(system(buf)) {r=1;goto out;}

+        sprintf(buf, "%s %s", SYS_SORT, o); if(system(buf)) {r=1;goto out;}

out:

        remove(c);

        remove(o);

 

From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=[hidden email]] On Behalf Of avih
Sent: Tuesday, June 18, 2019 21:01
To: [hidden email]; [hidden email]; 'Michael Matz'
Subject: Re: [Tinycc-devel] test 104 fails on windows: missing mkstemps

 

Well, the diffs are not really diffs. They just moved. Looks like `sort` didn't work as expected, or maybe it's some locale issue (mine is en_US.UTF-8 at cygwin, and en-US at windows).

 

A script should handle this too, possibly with LC_ALL=C (and make sure the reference file is generated with the same sort locale).

 

If someone could split it to program+script then I can test the test in various use cases and make adjustment if required (I'm terrible with Makefiles but reasonably useful with shell).

 

On Tuesday, June 18, 2019 9:50 PM, avih <[hidden email]> wrote:

 

After commit d39c49db  Remove empty conditional _WIN32 code



and some hacking of the code (it's an unhealthy mix of basically running a shell script from a program compiled using tcc for windows), I get the following 2 diffs:



+inst_extern_inline_postdeclared
+inst_extern_inline_predeclared



and



-inst_extern_inline_postdeclared
-inst_extern_inline_predeclared



I'm running it in a cygwin environment and the tools (nm, sort, gawk) are cygwin tools, while the tested tcc is normal mingw tcc for windows (which I build in a reproducible way using my script).



Regardless of these two diffs, I think the test should be composed of a program and a normal shell script (which uses mktemp, awk, sort etc), so that the paths are consistent between the tools.



Also, the TCC path is hardcoded at the test, but in fact it's parametric at the makefile as $(TCC), so it's better to use that instead (but then there are forward/backward slash issues which need to be handled too, because system(...) in win32 expects backward slashes, but $(TCC) at the makefile has forward slashes). Making this a program + a script should implicitly solve this issue as well.



After all, a working shell+tools is assumed for this test anyway, but the current way of using system(...) from a win32 program (compiled using tcc for windows) invokes a windows shell which can be inconsistent with the actual shell where `make` runs.



Avi





 

On Tuesday, June 18, 2019 12:11 AM, avih <[hidden email]> wrote:

 

Hmm.. I now see that test 104 uses signal and nm, so it might require some effort to make it work on windows.

 

Nevertheless, considering the recent breakage specifically on windows from related code, I think it would be beneficial if this test could be made to work on windows,

 

On Monday, June 17, 2019 11:54 PM, avih <[hidden email]> wrote:

 

Wouldn't it be better to just create a known/fixed file instead? (assuming the test doesn't need explicitly mkstemps, I didn't look at its actual code).

 

On Monday, June 17, 2019 11:50 PM, Christian Jullien <[hidden email]> wrote:

 

Yes it has been previously reported.

 

Michael, as said in a private mail, I agree with you that this test should be skipped on Windows.

 

C.

 

From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=[hidden email]] On Behalf Of avih
Sent: Monday, June 17, 2019 22:46
To: Tinycc-devel Mailing List
Subject: [Tinycc-devel] test 104 fails on windows: missing mkstemps

 

Test 104 log on windows (both tcc32 and tcc 64):

 

Test: 104_inline_test...
--- 104_inline_test.expect      2019-06-17 23:42:00.162697100 +0300
+++ 104_inline_test.output      2019-06-17 23:42:35.531550400 +0300
@@ -1,25 +1,2 @@
-extern_extern_postdeclared
-extern_extern_postdeclared2
-extern_extern_predeclared
-extern_extern_predeclared2
-extern_extern_prepostdeclared
-extern_extern_prepostdeclared2
-extern_extern_undeclared
-extern_extern_undeclared2
-extern_postdeclared
-extern_postdeclared2
-extern_predeclared
-extern_predeclared2
-extern_prepostdeclared
-extern_undeclared
-extern_undeclared2
-inst2_extern_inline_postdeclared
-inst2_extern_inline_predeclared
-inst3_extern_inline_predeclared
-inst_extern_inline_postdeclared
-inst_extern_inline_predeclared
-main
-noinst_extern_inline_func
-noinst_extern_inline_postdeclared
-noinst_extern_inline_postdeclared2
-noinst_extern_inline_undeclared
+104_inline_test.c:30: warning: implicit declaration of function 'mkstemps'
+tcc: error: undefined symbol 'mkstemps'
make[1]: *** [Makefile:70: 104_inline_test.test] Error 1
Test: 105_local_extern...
make[1]: Target 'all' not remade because of errors.

 

 

 

 

 


_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

avih
Hi Christian and thanks for the quick response.

I can confirm that your patch fixes the issue (makes the diff go away).

However, before the patch it definitely invoked the cygwin sort because my PATH only has cygwin's /bin and /local/bin (translated to correct windows paths when test 104 executes) and prefixed with TCC's root path. I tested this by printing getenv("PATH") at 104_inline_test.c .

I can also confirm the issue simply from cygwin's shell that `sort` changes the order of 104_inline_test.expect:
$ cat tests/tests2/104_inline_test.expect | sort # <-- inst_... is before inst2_... i.e. MODIFIED.

while

$ cat tests/tests2/104_inline_test.expect | LC_ALL=C sort # <-- inst2... is before inst_...  i.e. unmodified = OK

Alternative to your patch, adding `export LC_ALL = C` at the beginning of tests/tests2/Makefile also fixes the issue for me.

Running `locale` at the shell prints this for me:
$ locale
LANG=en_US.UTF-8
LC_CTYPE="en_US.UTF-8"
LC_NUMERIC="en_US.UTF-8"
LC_TIME="en_US.UTF-8"
LC_COLLATE="en_US.UTF-8"
LC_MONETARY="en_US.UTF-8"
LC_MESSAGES="en_US.UTF-8"
LC_ALL=

Please don't push this patch to mob, because it adds another potential complexity by mixing unix/windows tools.

I'll try to modify 104_inline_test.c such that it uses only one system invocation of shell -c "<commands>" so that it effectively becomes a shell script, and post the result of my attempt here to get some feedback.

Thanks again,
Avi


On Wednesday, June 19, 2019 8:20 AM, Christian Jullien <[hidden email]> wrote:


Avih,
I quickly hacked 104 test to start to make it work but I’m not the author of this test. I let Petr improve it.
I fully agree with you that mixing C code and calling system (which forks a cmd.exe on Windows) to finally execute gnu commands that require Cygwin is a BAD idea!!
As you said, it’s much easier if 104 test generates a .o which the whole ‘unix’ infrastructure test will check. I let the maintainer of this test decide what to do.
 
About the diff, I don’t understand quite well what happens. Among other, it calls system(“sort …”); which spawns cmd.exe and then execute first sort seen in path. Depending on your PATH it can be Windows sort (if Windows path comes first) or Cygwin version if this one comes first.
 
Can you please try this diff which works as good for me but now forces sort Windows version:
 
diff --git a/tests/tests2/104_inline_test.c b/tests/tests2/104_inline_test.c
index cb288d2..d191602 100644
--- a/tests/tests2/104_inline_test.c
+++ b/tests/tests2/104_inline_test.c
@@ -5,8 +5,8 @@
 
#if __linux__ || __APPLE__
#define SYS_WHICH_NM "which nm >/dev/null 2>&1"
+#define SYS_SORT     "sort"
#define TCC_COMPILER "../../tcc"
-#define SYS_AWK
 
char c[]="/tmp/tcc-XXXXXX"; char o[]="/tmp/tcc-XXXXXX";
static int mktempfile(char *buf)
@@ -15,6 +15,7 @@ static int mktempfile(char *buf)
}
#elif defined(_WIN32)
#define SYS_WHICH_NM  "which nm >nul 2>&1"
+#define SYS_SORT      "%systemroot%\\system32\\sort /LOCALE C"
 
#if defined(_WIN64)
#define TCC_COMPILER "..\\..\\win32\\x86_64-win32-tcc"
@@ -72,7 +73,7 @@ int main(int C, char **V)
         sprintf(buf, "%s -c -xc %s -o %s", V[1]?V[1]:TCC_COMPILER, c, o); if(0!=system(buf)){ r=1;goto out;}
         sprintf(buf, "nm -Ptx %s > %s", o, c); if(system(buf)) {r=1;goto out;}
         sprintf(buf, "gawk '{ if($2 == \"T\") print $1 }' %s > %s", c, o); if(system(buf)) {r=1;goto out;}
-        sprintf(buf, "sort %s", o); if(system(buf)) {r=1;goto out;}
+        sprintf(buf, "%s %s", SYS_SORT, o); if(system(buf)) {r=1;goto out;}
out:
        remove(c);
        remove(o);
 
From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=[hidden email]] On Behalf Of avih
Sent: Tuesday, June 18, 2019 21:01
To: [hidden email]; [hidden email]; 'Michael Matz'
Subject: Re: [Tinycc-devel] test 104 fails on windows: missing mkstemps
 
Well, the diffs are not really diffs. They just moved. Looks like `sort` didn't work as expected, or maybe it's some locale issue (mine is en_US.UTF-8 at cygwin, and en-US at windows).
 
A script should handle this too, possibly with LC_ALL=C (and make sure the reference file is generated with the same sort locale).
 
If someone could split it to program+script then I can test the test in various use cases and make adjustment if required (I'm terrible with Makefiles but reasonably useful with shell).
 
On Tuesday, June 18, 2019 9:50 PM, avih <[hidden email]> wrote:
 
After commit d39c49db  Remove empty conditional _WIN32 code


and some hacking of the code (it's an unhealthy mix of basically running a shell script from a program compiled using tcc for windows), I get the following 2 diffs:


+inst_extern_inline_postdeclared
+inst_extern_inline_predeclared



and


-inst_extern_inline_postdeclared
-inst_extern_inline_predeclared



I'm running it in a cygwin environment and the tools (nm, sort, gawk) are cygwin tools, while the tested tcc is normal mingw tcc for windows (which I build in a reproducible way using my script).



Regardless of these two diffs, I think the test should be composed of a program and a normal shell script (which uses mktemp, awk, sort etc), so that the paths are consistent between the tools.


Also, the TCC path is hardcoded at the test, but in fact it's parametric at the makefile as $(TCC), so it's better to use that instead (but then there are forward/backward slash issues which need to be handled too, because system(...) in win32 expects backward slashes, but $(TCC) at the makefile has forward slashes). Making this a program + a script should implicitly solve this issue as well.



After all, a working shell+tools is assumed for this test anyway, but the current way of using system(...) from a win32 program (compiled using tcc for windows) invokes a windows shell which can be inconsistent with the actual shell where `make` runs.


Avi





 
On Tuesday, June 18, 2019 12:11 AM, avih <[hidden email]> wrote:
 
Hmm.. I now see that test 104 uses signal and nm, so it might require some effort to make it work on windows.
 
Nevertheless, considering the recent breakage specifically on windows from related code, I think it would be beneficial if this test could be made to work on windows,
 
On Monday, June 17, 2019 11:54 PM, avih <[hidden email]> wrote:
 
Wouldn't it be better to just create a known/fixed file instead? (assuming the test doesn't need explicitly mkstemps, I didn't look at its actual code).
 
On Monday, June 17, 2019 11:50 PM, Christian Jullien <[hidden email]> wrote:
 
Yes it has been previously reported.
 
Michael, as said in a private mail, I agree with you that this test should be skipped on Windows.
 
C.
 
From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=[hidden email]] On Behalf Of avih
Sent: Monday, June 17, 2019 22:46
To: Tinycc-devel Mailing List
Subject: [Tinycc-devel] test 104 fails on windows: missing mkstemps
 
Test 104 log on windows (both tcc32 and tcc 64):
 
Test: 104_inline_test...
--- 104_inline_test.expect      2019-06-17 23:42:00.162697100 +0300
+++ 104_inline_test.output      2019-06-17 23:42:35.531550400 +0300
@@ -1,25 +1,2 @@
-extern_extern_postdeclared
-extern_extern_postdeclared2
-extern_extern_predeclared
-extern_extern_predeclared2
-extern_extern_prepostdeclared
-extern_extern_prepostdeclared2
-extern_extern_undeclared
-extern_extern_undeclared2
-extern_postdeclared
-extern_postdeclared2
-extern_predeclared
-extern_predeclared2
-extern_prepostdeclared
-extern_undeclared
-extern_undeclared2
-inst2_extern_inline_postdeclared
-inst2_extern_inline_predeclared
-inst3_extern_inline_predeclared
-inst_extern_inline_postdeclared
-inst_extern_inline_predeclared
-main
-noinst_extern_inline_func
-noinst_extern_inline_postdeclared
-noinst_extern_inline_postdeclared2
-noinst_extern_inline_undeclared
+104_inline_test.c:30: warning: implicit declaration of function 'mkstemps'
+tcc: error: undefined symbol 'mkstemps'
make[1]: *** [Makefile:70: 104_inline_test.test] Error 1
Test: 105_local_extern...
make[1]: Target 'all' not remade because of errors.
 
 
 
 
 



_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

Christian Jullien-3

Ok I let you modify this test. When done, I’ll be glad to test it and tell you if it still works.

Many times I’ve seen that shell scripts also better work with LC_ALL=C.

Unless you’re prepared to support every possible LC_XXX settings, LC_ALL=C is very often our best friend.

Among other, French locale should interpret float with a comma (as 3,14 instead of 3.14). Needless to say that it breaks uncounted number of programs.

 

C.

 

From: avih [mailto:[hidden email]]
Sent: Wednesday, June 19, 2019 11:33
To: [hidden email]; [hidden email]; 'Michael Matz'; 'Petr Skočík'
Subject: Re: [Tinycc-devel] test 104 fails on windows: missing mkstemps

 

Hi Christian and thanks for the quick response.



I can confirm that your patch fixes the issue (makes the diff go away).



However, before the patch it definitely invoked the cygwin sort because my PATH only has cygwin's /bin and /local/bin (translated to correct windows paths when test 104 executes) and prefixed with TCC's root path. I tested this by printing getenv("PATH") at 104_inline_test.c .



I can also confirm the issue simply from cygwin's shell that `sort` changes the order of 104_inline_test.expect:

$ cat tests/tests2/104_inline_test.expect | sort # <-- inst_... is before inst2_... i.e. MODIFIED.



while


$ cat tests/tests2/104_inline_test.expect | LC_ALL=C sort # <-- inst2... is before inst_...  i.e. unmodified = OK



Alternative to your patch, adding `export LC_ALL = C` at the beginning of tests/tests2/Makefile also fixes the issue for me.



Running `locale` at the shell prints this for me:

$ locale
LANG=en_US.UTF-8
LC_CTYPE="en_US.UTF-8"
LC_NUMERIC="en_US.UTF-8"
LC_TIME="en_US.UTF-8"
LC_COLLATE="en_US.UTF-8"
LC_MONETARY="en_US.UTF-8"
LC_MESSAGES="en_US.UTF-8"
LC_ALL=



Please don't push this patch to mob, because it adds another potential complexity by mixing unix/windows tools.



I'll try to modify 104_inline_test.c such that it uses only one system invocation of shell -c "<commands>" so that it effectively becomes a shell script, and post the result of my attempt here to get some feedback.



Thanks again,

Avi

 

On Wednesday, June 19, 2019 8:20 AM, Christian Jullien <[hidden email]> wrote:

 

Avih,

I quickly hacked 104 test to start to make it work but I’m not the author of this test. I let Petr improve it.

I fully agree with you that mixing C code and calling system (which forks a cmd.exe on Windows) to finally execute gnu commands that require Cygwin is a BAD idea!!

As you said, it’s much easier if 104 test generates a .o which the whole ‘unix’ infrastructure test will check. I let the maintainer of this test decide what to do.

 

About the diff, I don’t understand quite well what happens. Among other, it calls system(“sort …”); which spawns cmd.exe and then execute first sort seen in path. Depending on your PATH it can be Windows sort (if Windows path comes first) or Cygwin version if this one comes first.

 

Can you please try this diff which works as good for me but now forces sort Windows version:

 

diff --git a/tests/tests2/104_inline_test.c b/tests/tests2/104_inline_test.c

index cb288d2..d191602 100644

--- a/tests/tests2/104_inline_test.c

+++ b/tests/tests2/104_inline_test.c

@@ -5,8 +5,8 @@

 

#if __linux__ || __APPLE__

#define SYS_WHICH_NM "which nm >/dev/null 2>&1"

+#define SYS_SORT     "sort"

#define TCC_COMPILER "../../tcc"

-#define SYS_AWK

 

char c[]="/tmp/tcc-XXXXXX"; char o[]="/tmp/tcc-XXXXXX";

static int mktempfile(char *buf)

@@ -15,6 +15,7 @@ static int mktempfile(char *buf)

}

#elif defined(_WIN32)

#define SYS_WHICH_NM  "which nm >nul 2>&1"

+#define SYS_SORT      "%systemroot%\\system32\\sort /LOCALE C"

 

#if defined(_WIN64)

#define TCC_COMPILER "..\\..\\win32\\x86_64-win32-tcc"

@@ -72,7 +73,7 @@ int main(int C, char **V)

         sprintf(buf, "%s -c -xc %s -o %s", V[1]?V[1]:TCC_COMPILER, c, o); if(0!=system(buf)){ r=1;goto out;}

         sprintf(buf, "nm -Ptx %s > %s", o, c); if(system(buf)) {r=1;goto out;}

         sprintf(buf, "gawk '{ if($2 == \"T\") print $1 }' %s > %s", c, o); if(system(buf)) {r=1;goto out;}

-        sprintf(buf, "sort %s", o); if(system(buf)) {r=1;goto out;}

+        sprintf(buf, "%s %s", SYS_SORT, o); if(system(buf)) {r=1;goto out;}

out:

        remove(c);

        remove(o);

 

From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=[hidden email]] On Behalf Of avih
Sent: Tuesday, June 18, 2019 21:01
To: [hidden email]; [hidden email]; 'Michael Matz'
Subject: Re: [Tinycc-devel] test 104 fails on windows: missing mkstemps

 

Well, the diffs are not really diffs. They just moved. Looks like `sort` didn't work as expected, or maybe it's some locale issue (mine is en_US.UTF-8 at cygwin, and en-US at windows).

 

A script should handle this too, possibly with LC_ALL=C (and make sure the reference file is generated with the same sort locale).

 

If someone could split it to program+script then I can test the test in various use cases and make adjustment if required (I'm terrible with Makefiles but reasonably useful with shell).

 

On Tuesday, June 18, 2019 9:50 PM, avih <[hidden email]> wrote:

 

After commit d39c49db  Remove empty conditional _WIN32 code

 

and some hacking of the code (it's an unhealthy mix of basically running a shell script from a program compiled using tcc for windows), I get the following 2 diffs:

 

+inst_extern_inline_postdeclared
+inst_extern_inline_predeclared

 

and

 

-inst_extern_inline_postdeclared
-inst_extern_inline_predeclared

 

I'm running it in a cygwin environment and the tools (nm, sort, gawk) are cygwin tools, while the tested tcc is normal mingw tcc for windows (which I build in a reproducible way using my script).

 

Regardless of these two diffs, I think the test should be composed of a program and a normal shell script (which uses mktemp, awk, sort etc), so that the paths are consistent between the tools.

 

Also, the TCC path is hardcoded at the test, but in fact it's parametric at the makefile as $(TCC), so it's better to use that instead (but then there are forward/backward slash issues which need to be handled too, because system(...) in win32 expects backward slashes, but $(TCC) at the makefile has forward slashes). Making this a program + a script should implicitly solve this issue as well.

 

After all, a working shell+tools is assumed for this test anyway, but the current way of using system(...) from a win32 program (compiled using tcc for windows) invokes a windows shell which can be inconsistent with the actual shell where `make` runs.

 

Avi

 

 

 

On Tuesday, June 18, 2019 12:11 AM, avih <[hidden email]> wrote:

 

Hmm.. I now see that test 104 uses signal and nm, so it might require some effort to make it work on windows.

 

Nevertheless, considering the recent breakage specifically on windows from related code, I think it would be beneficial if this test could be made to work on windows,

 

On Monday, June 17, 2019 11:54 PM, avih <[hidden email]> wrote:

 

Wouldn't it be better to just create a known/fixed file instead? (assuming the test doesn't need explicitly mkstemps, I didn't look at its actual code).

 

On Monday, June 17, 2019 11:50 PM, Christian Jullien <[hidden email]> wrote:

 

Yes it has been previously reported.

 

Michael, as said in a private mail, I agree with you that this test should be skipped on Windows.

 

C.

 

From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=[hidden email]] On Behalf Of avih
Sent: Monday, June 17, 2019 22:46
To: Tinycc-devel Mailing List
Subject: [Tinycc-devel] test 104 fails on windows: missing mkstemps

 

Test 104 log on windows (both tcc32 and tcc 64):

 

Test: 104_inline_test...
--- 104_inline_test.expect      2019-06-17 23:42:00.162697100 +0300
+++ 104_inline_test.output      2019-06-17 23:42:35.531550400 +0300
@@ -1,25 +1,2 @@
-extern_extern_postdeclared
-extern_extern_postdeclared2
-extern_extern_predeclared
-extern_extern_predeclared2
-extern_extern_prepostdeclared
-extern_extern_prepostdeclared2
-extern_extern_undeclared
-extern_extern_undeclared2
-extern_postdeclared
-extern_postdeclared2
-extern_predeclared
-extern_predeclared2
-extern_prepostdeclared
-extern_undeclared
-extern_undeclared2
-inst2_extern_inline_postdeclared
-inst2_extern_inline_predeclared
-inst3_extern_inline_predeclared
-inst_extern_inline_postdeclared
-inst_extern_inline_predeclared
-main
-noinst_extern_inline_func
-noinst_extern_inline_postdeclared
-noinst_extern_inline_postdeclared2
-noinst_extern_inline_undeclared
+104_inline_test.c:30: warning: implicit declaration of function 'mkstemps'
+tcc: error: undefined symbol 'mkstemps'
make[1]: *** [Makefile:70: 104_inline_test.test] Error 1
Test: 105_local_extern...
make[1]: Target 'all' not remade because of errors.

 

 

 

 

 

 


_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

Petr Skočík
In reply to this post by avih
The test started out as a shell script. It could have been a pipeline,
but what I dislike about pipelines in POSIX shell is you can't
really error check them well because if a link fails and the last one
succeeds, you get a success.

That is why I wrote I ended up invoking the links from C
using tempfiles as glue. You can theoretically do the same from a shell
but mktemp isn't standard but you can create tempfiles from C standardly
(mkstemps() is POSIX, tmpnam() is ISO C) and the C code isn't much
longer than the shell code.

Petr

On 6/19/19 11:33 AM, avih wrote:

> Hi Christian and thanks for the quick response.
>
> I can confirm that your patch fixes the issue (makes the diff go away).
>
> However, before the patch it definitely invoked the cygwin sort because
> my PATH only has cygwin's /bin and /local/bin (translated to correct
> windows paths when test 104 executes) and prefixed with TCC's root path.
> I tested this by printing getenv("PATH") at 104_inline_test.c .
>
> I can also confirm the issue simply from cygwin's shell that `sort`
> changes the order of 104_inline_test.expect:
> $ cat tests/tests2/104_inline_test.expect | sort # <-- inst_... is
> before inst2_... i.e. MODIFIED.
>
> while
>
> $ cat tests/tests2/104_inline_test.expect | LC_ALL=C sort # <-- inst2...
> is before inst_...  i.e. unmodified = OK
>
> Alternative to your patch, adding `export LC_ALL = C` at the beginning
> of tests/tests2/Makefile also fixes the issue for me.
>
> Running `locale` at the shell prints this for me:
> $ locale
> LANG=en_US.UTF-8
> LC_CTYPE="en_US.UTF-8"
> LC_NUMERIC="en_US.UTF-8"
> LC_TIME="en_US.UTF-8"
> LC_COLLATE="en_US.UTF-8"
> LC_MONETARY="en_US.UTF-8"
> LC_MESSAGES="en_US.UTF-8"
> LC_ALL=
>
> Please don't push this patch to mob, because it adds another potential
> complexity by mixing unix/windows tools.
>
> I'll try to modify 104_inline_test.c such that it uses only one system
> invocation of shell -c "<commands>" so that it effectively becomes a
> shell script, and post the result of my attempt here to get some feedback.
>
> Thanks again,
> Avi
>
>
> On Wednesday, June 19, 2019 8:20 AM, Christian Jullien
> <[hidden email]> wrote:
>
>
> Avih,
> I quickly hacked 104 test to start to make it work but I’m not the
> author of this test. I let Petr improve it.
> I fully agree with you that mixing C code and calling system (which
> forks a cmd.exe on Windows) to finally execute gnu commands that require
> Cygwin is a BAD idea!!
> As you said, it’s much easier if 104 test generates a .o which the whole
> ‘unix’ infrastructure test will check. I let the maintainer of this test
> decide what to do.
>  
> About the diff, I don’t understand quite well what happens. Among other,
> it calls system(“sort …”); which spawns cmd.exe and then execute first
> sort seen in path. Depending on your PATH it can be Windows sort (if
> Windows path comes first) or Cygwin version if this one comes first.
>  
> Can you please try this diff which works as good for me but now forces
> sort Windows version:
>  
> diff --git a/tests/tests2/104_inline_test.c b/tests/tests2/104_inline_test.c
> index cb288d2..d191602 100644
> --- a/tests/tests2/104_inline_test.c
> +++ b/tests/tests2/104_inline_test.c
> @@ -5,8 +5,8 @@
>  
> #if __linux__ || __APPLE__
> #define SYS_WHICH_NM "which nm >/dev/null 2>&1"
> +#define SYS_SORT     "sort"
> #define TCC_COMPILER "../../tcc"
> -#define SYS_AWK
>  
> char c[]="/tmp/tcc-XXXXXX"; char o[]="/tmp/tcc-XXXXXX";
> static int mktempfile(char *buf)
> @@ -15,6 +15,7 @@ static int mktempfile(char *buf)
> }
> #elif defined(_WIN32)
> #define SYS_WHICH_NM  "which nm >nul 2>&1"
> +#define SYS_SORT      "%systemroot%\\system32\\sort /LOCALE C"
>  
> #if defined(_WIN64)
> #define TCC_COMPILER "..\\..\\win32\\x86_64-win32-tcc"
> @@ -72,7 +73,7 @@ int main(int C, char **V)
>          sprintf(buf, "%s -c -xc %s -o %s", V[1]?V[1]:TCC_COMPILER, c,
> o); if(0!=system(buf)){ r=1;goto out;}
>          sprintf(buf, "nm -Ptx %s > %s", o, c); if(system(buf))
> {r=1;goto out;}
>          sprintf(buf, "gawk '{ if($2 == \"T\") print $1 }' %s > %s", c,
> o); if(system(buf)) {r=1;goto out;}
> -        sprintf(buf, "sort %s", o); if(system(buf)) {r=1;goto out;}
> +        sprintf(buf, "%s %s", SYS_SORT, o); if(system(buf)) {r=1;goto out;}
> out:
>         remove(c);
>         remove(o);
>  
> *From:*Tinycc-devel
> [mailto:tinycc-devel-bounces+eligis=[hidden email]] *On Behalf Of
> *avih
> *Sent:* Tuesday, June 18, 2019 21:01
> *To:* [hidden email]; [hidden email]; 'Michael Matz'
> *Subject:* Re: [Tinycc-devel] test 104 fails on windows: missing mkstemps
>  
> Well, the diffs are not really diffs. They just moved. Looks like `sort`
> didn't work as expected, or maybe it's some locale issue (mine is
> en_US.UTF-8 at cygwin, and en-US at windows).
>  
> A script should handle this too, possibly with LC_ALL=C (and make sure
> the reference file is generated with the same sort locale).
>  
> If someone could split it to program+script then I can test the test in
> various use cases and make adjustment if required (I'm terrible with
> Makefiles but reasonably useful with shell).
>  
> On Tuesday, June 18, 2019 9:50 PM, avih <[hidden email]> wrote:
>  
> After commit d39c49db  Remove empty conditional _WIN32 code
>
>
> and some hacking of the code (it's an unhealthy mix of basically running
> a shell script from a program compiled using tcc for windows), I get the
> following 2 diffs:
>
>
> +inst_extern_inline_postdeclared
> +inst_extern_inline_predeclared
>
>
>
> and
>
>
> -inst_extern_inline_postdeclared
> -inst_extern_inline_predeclared
>
>
>
> I'm running it in a cygwin environment and the tools (nm, sort, gawk)
> are cygwin tools, while the tested tcc is normal mingw tcc for windows
> (which I build in a reproducible way using my script).
>
>
>
> Regardless of these two diffs, I think the test should be composed of a
> program and a normal shell script (which uses mktemp, awk, sort etc), so
> that the paths are consistent between the tools.
>
>
> Also, the TCC path is hardcoded at the test, but in fact it's parametric
> at the makefile as $(TCC), so it's better to use that instead (but then
> there are forward/backward slash issues which need to be handled too,
> because system(...) in win32 expects backward slashes, but $(TCC) at the
> makefile has forward slashes). Making this a program + a script should
> implicitly solve this issue as well.
>
>
>
> After all, a working shell+tools is assumed for this test anyway, but
> the current way of using system(...) from a win32 program (compiled
> using tcc for windows) invokes a windows shell which can be inconsistent
> with the actual shell where `make` runs.
>
>
> Avi
>
>
>
>
>
>  
> On Tuesday, June 18, 2019 12:11 AM, avih <[hidden email]> wrote:
>  
> Hmm.. I now see that test 104 uses signal and nm, so it might require
> some effort to make it work on windows.
>  
> Nevertheless, considering the recent breakage specifically on windows
> from related code, I think it would be beneficial if this test could be
> made to work on windows,
>  
> On Monday, June 17, 2019 11:54 PM, avih <[hidden email]> wrote:
>  
> Wouldn't it be better to just create a known/fixed file instead?
> (assuming the test doesn't need explicitly mkstemps, I didn't look at
> its actual code).
>  
> On Monday, June 17, 2019 11:50 PM, Christian Jullien <[hidden email]>
> wrote:
>  
> Yes it has been previously reported.
>  
> Michael, as said in a private mail, I agree with you that this test
> should be skipped on Windows.
>  
> C.
>  
> *From:*Tinycc-devel
> [mailto:tinycc-devel-bounces+eligis=[hidden email]] *On Behalf Of
> *avih
> *Sent:* Monday, June 17, 2019 22:46
> *To:* Tinycc-devel Mailing List
> *Subject:* [Tinycc-devel] test 104 fails on windows: missing mkstemps
>  
> Test 104 log on windows (both tcc32 and tcc 64):
>  
> Test: 104_inline_test...
> --- 104_inline_test.expect      2019-06-17 23:42:00.162697100 +0300
> +++ 104_inline_test.output      2019-06-17 23:42:35.531550400 +0300
> @@ -1,25 +1,2 @@
> -extern_extern_postdeclared
> -extern_extern_postdeclared2
> -extern_extern_predeclared
> -extern_extern_predeclared2
> -extern_extern_prepostdeclared
> -extern_extern_prepostdeclared2
> -extern_extern_undeclared
> -extern_extern_undeclared2
> -extern_postdeclared
> -extern_postdeclared2
> -extern_predeclared
> -extern_predeclared2
> -extern_prepostdeclared
> -extern_undeclared
> -extern_undeclared2
> -inst2_extern_inline_postdeclared
> -inst2_extern_inline_predeclared
> -inst3_extern_inline_predeclared
> -inst_extern_inline_postdeclared
> -inst_extern_inline_predeclared
> -main
> -noinst_extern_inline_func
> -noinst_extern_inline_postdeclared
> -noinst_extern_inline_postdeclared2
> -noinst_extern_inline_undeclared
> +104_inline_test.c:30: warning: implicit declaration of function 'mkstemps'
> +tcc: error: undefined symbol 'mkstemps'
> make[1]: *** [Makefile:70: 104_inline_test.test] Error 1
> Test: 105_local_extern...
> make[1]: Target 'all' not remade because of errors.
>  
>  
>  
>  
>  
>
>


_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

avih
Peter,

You're correct, but on windows tcc isn't posix compliant, hence no mkstemp.

Additionally, `system(...)` on windows invokes the windows shell, which typically is not the shell where `make` is invoked (and which make uses), and it has different path expectations than the test shell.

As for pipe failure, you could (and I'm currently doing while working on it) use the posix `set -e` and execute the commands individually while still failing on errors. I'm aware that `set -e` can be a can of worms, but for this simple script it doesn't pose hidden pitfalls that I can identify.

As for `mktemp` not being standard, I wasn't aware of that, thanks. For now I'll use it, but I won't post it for review before I have a reasonable alternative if it's not available.

Alternatively, if you have a version which uses a shell script (and the tcc path taken from the TCC env var which the makefile sets anyway) then you can push it instead of me keep working on it.

Avi


On Wednesday, June 19, 2019 1:17 PM, Petr Skočík <[hidden email]> wrote:


The test started out as a shell script. It could have been a pipeline,
but what I dislike about pipelines in POSIX shell is you can't
really error check them well because if a link fails and the last one
succeeds, you get a success.

That is why I wrote I ended up invoking the links from C
using tempfiles as glue. You can theoretically do the same from a shell
but mktemp isn't standard but you can create tempfiles from C standardly
(mkstemps() is POSIX, tmpnam() is ISO C) and the C code isn't much
longer than the shell code.

Petr

On 6/19/19 11:33 AM, avih wrote:

> Hi Christian and thanks for the quick response.
>
> I can confirm that your patch fixes the issue (makes the diff go away).
>
> However, before the patch it definitely invoked the cygwin sort because
> my PATH only has cygwin's /bin and /local/bin (translated to correct
> windows paths when test 104 executes) and prefixed with TCC's root path.
> I tested this by printing getenv("PATH") at 104_inline_test.c .
>
> I can also confirm the issue simply from cygwin's shell that `sort`
> changes the order of 104_inline_test.expect:
> $ cat tests/tests2/104_inline_test.expect | sort # <-- inst_... is
> before inst2_... i.e. MODIFIED.
>
> while
>
> $ cat tests/tests2/104_inline_test.expect | LC_ALL=C sort # <-- inst2...
> is before inst_...  i.e. unmodified = OK
>
> Alternative to your patch, adding `export LC_ALL = C` at the beginning
> of tests/tests2/Makefile also fixes the issue for me.
>
> Running `locale` at the shell prints this for me:
> $ locale
> LANG=en_US.UTF-8
> LC_CTYPE="en_US.UTF-8"
> LC_NUMERIC="en_US.UTF-8"
> LC_TIME="en_US.UTF-8"
> LC_COLLATE="en_US.UTF-8"
> LC_MONETARY="en_US.UTF-8"
> LC_MESSAGES="en_US.UTF-8"
> LC_ALL=
>
> Please don't push this patch to mob, because it adds another potential
> complexity by mixing unix/windows tools.
>
> I'll try to modify 104_inline_test.c such that it uses only one system
> invocation of shell -c "<commands>" so that it effectively becomes a
> shell script, and post the result of my attempt here to get some feedback.
>
> Thanks again,
> Avi
>
>
> On Wednesday, June 19, 2019 8:20 AM, Christian Jullien
> <[hidden email]> wrote:
>
>
> Avih,
> I quickly hacked 104 test to start to make it work but I’m not the
> author of this test. I let Petr improve it.
> I fully agree with you that mixing C code and calling system (which
> forks a cmd.exe on Windows) to finally execute gnu commands that require
> Cygwin is a BAD idea!!
> As you said, it’s much easier if 104 test generates a .o which the whole
> ‘unix’ infrastructure test will check. I let the maintainer of this test
> decide what to do.
>  
> About the diff, I don’t understand quite well what happens. Among other,
> it calls system(“sort …”); which spawns cmd.exe and then execute first
> sort seen in path. Depending on your PATH it can be Windows sort (if
> Windows path comes first) or Cygwin version if this one comes first.
>  
> Can you please try this diff which works as good for me but now forces
> sort Windows version:
>  
> diff --git a/tests/tests2/104_inline_test.c b/tests/tests2/104_inline_test.c
> index cb288d2..d191602 100644
> --- a/tests/tests2/104_inline_test.c
> +++ b/tests/tests2/104_inline_test.c
> @@ -5,8 +5,8 @@
>  
> #if __linux__ || __APPLE__
> #define SYS_WHICH_NM "which nm >/dev/null 2>&1"
> +#define SYS_SORT     "sort"
> #define TCC_COMPILER "../../tcc"
> -#define SYS_AWK
>  
> char c[]="/tmp/tcc-XXXXXX"; char o[]="/tmp/tcc-XXXXXX";
> static int mktempfile(char *buf)
> @@ -15,6 +15,7 @@ static int mktempfile(char *buf)
> }
> #elif defined(_WIN32)
> #define SYS_WHICH_NM  "which nm >nul 2>&1"
> +#define SYS_SORT      "%systemroot%\\system32\\sort /LOCALE C"
>  
> #if defined(_WIN64)
> #define TCC_COMPILER "..\\..\\win32\\x86_64-win32-tcc"
> @@ -72,7 +73,7 @@ int main(int C, char **V)
>          sprintf(buf, "%s -c -xc %s -o %s", V[1]?V[1]:TCC_COMPILER, c,
> o); if(0!=system(buf)){ r=1;goto out;}
>          sprintf(buf, "nm -Ptx %s > %s", o, c); if(system(buf))
> {r=1;goto out;}
>          sprintf(buf, "gawk '{ if($2 == \"T\") print $1 }' %s > %s", c,
> o); if(system(buf)) {r=1;goto out;}
> -        sprintf(buf, "sort %s", o); if(system(buf)) {r=1;goto out;}
> +        sprintf(buf, "%s %s", SYS_SORT, o); if(system(buf)) {r=1;goto out;}
> out:
>         remove(c);
>         remove(o);
>  
> *From:*Tinycc-devel
> [mailto:tinycc-devel-bounces+eligis=[hidden email]] *On Behalf Of
> *avih
> *Sent:* Tuesday, June 18, 2019 21:01
> *To:* [hidden email]; [hidden email]; 'Michael Matz'
> *Subject:* Re: [Tinycc-devel] test 104 fails on windows: missing mkstemps
>  
> Well, the diffs are not really diffs. They just moved. Looks like `sort`
> didn't work as expected, or maybe it's some locale issue (mine is
> en_US.UTF-8 at cygwin, and en-US at windows).
>  
> A script should handle this too, possibly with LC_ALL=C (and make sure
> the reference file is generated with the same sort locale).
>  
> If someone could split it to program+script then I can test the test in
> various use cases and make adjustment if required (I'm terrible with
> Makefiles but reasonably useful with shell).
>  
> On Tuesday, June 18, 2019 9:50 PM, avih <[hidden email]> wrote:
>  
> After commit d39c49db  Remove empty conditional _WIN32 code
>
>
> and some hacking of the code (it's an unhealthy mix of basically running
> a shell script from a program compiled using tcc for windows), I get the
> following 2 diffs:
>
>
> +inst_extern_inline_postdeclared
> +inst_extern_inline_predeclared
>
>
>
> and
>
>
> -inst_extern_inline_postdeclared
> -inst_extern_inline_predeclared
>
>
>
> I'm running it in a cygwin environment and the tools (nm, sort, gawk)
> are cygwin tools, while the tested tcc is normal mingw tcc for windows
> (which I build in a reproducible way using my script).
>
>
>
> Regardless of these two diffs, I think the test should be composed of a
> program and a normal shell script (which uses mktemp, awk, sort etc), so
> that the paths are consistent between the tools.
>
>
> Also, the TCC path is hardcoded at the test, but in fact it's parametric
> at the makefile as $(TCC), so it's better to use that instead (but then
> there are forward/backward slash issues which need to be handled too,
> because system(...) in win32 expects backward slashes, but $(TCC) at the
> makefile has forward slashes). Making this a program + a script should
> implicitly solve this issue as well.
>
>
>
> After all, a working shell+tools is assumed for this test anyway, but
> the current way of using system(...) from a win32 program (compiled
> using tcc for windows) invokes a windows shell which can be inconsistent
> with the actual shell where `make` runs.
>
>
> Avi
>
>
>
>
>
>  
> On Tuesday, June 18, 2019 12:11 AM, avih <[hidden email]> wrote:
>  
> Hmm.. I now see that test 104 uses signal and nm, so it might require
> some effort to make it work on windows.
>  
> Nevertheless, considering the recent breakage specifically on windows
> from related code, I think it would be beneficial if this test could be
> made to work on windows,
>  
> On Monday, June 17, 2019 11:54 PM, avih <[hidden email]> wrote:
>  
> Wouldn't it be better to just create a known/fixed file instead?
> (assuming the test doesn't need explicitly mkstemps, I didn't look at
> its actual code).
>  
> On Monday, June 17, 2019 11:50 PM, Christian Jullien <[hidden email]>
> wrote:
>  
> Yes it has been previously reported.
>  
> Michael, as said in a private mail, I agree with you that this test
> should be skipped on Windows.
>  
> C.
>  
> *From:*Tinycc-devel
> [mailto:tinycc-devel-bounces+eligis=[hidden email]] *On Behalf Of
> *avih
> *Sent:* Monday, June 17, 2019 22:46
> *To:* Tinycc-devel Mailing List
> *Subject:* [Tinycc-devel] test 104 fails on windows: missing mkstemps
>  
> Test 104 log on windows (both tcc32 and tcc 64):
>  
> Test: 104_inline_test...
> --- 104_inline_test.expect      2019-06-17 23:42:00.162697100 +0300
> +++ 104_inline_test.output      2019-06-17 23:42:35.531550400 +0300
> @@ -1,25 +1,2 @@
> -extern_extern_postdeclared
> -extern_extern_postdeclared2
> -extern_extern_predeclared
> -extern_extern_predeclared2
> -extern_extern_prepostdeclared
> -extern_extern_prepostdeclared2
> -extern_extern_undeclared
> -extern_extern_undeclared2
> -extern_postdeclared
> -extern_postdeclared2
> -extern_predeclared
> -extern_predeclared2
> -extern_prepostdeclared
> -extern_undeclared
> -extern_undeclared2
> -inst2_extern_inline_postdeclared
> -inst2_extern_inline_predeclared
> -inst3_extern_inline_predeclared
> -inst_extern_inline_postdeclared
> -inst_extern_inline_predeclared
> -main
> -noinst_extern_inline_func
> -noinst_extern_inline_postdeclared
> -noinst_extern_inline_postdeclared2
> -noinst_extern_inline_undeclared
> +104_inline_test.c:30: warning: implicit declaration of function 'mkstemps'
> +tcc: error: undefined symbol 'mkstemps'
> make[1]: *** [Makefile:70: 104_inline_test.test] Error 1
> Test: 105_local_extern...
> make[1]: Target 'all' not remade because of errors.
>  
>  
>  
>  
>  
>
>




_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

avih
If no objects to the attached patch, then I'll push it to mob.

In a nutshell: it creates and executes a shell script, where the tcc binary path (and arguments) are taken from the Makefile (where it `export TESTED_TCC = $(TCC)`). It was tested on Ubuntu and Windows (cygwin).

While I think it's better than before, it's still rather awkward, and at some stage we'll probably want to compile cFileContents directly from the makefile and then execute the commands at the script also at the Makefile.

The patch replaces everything before cFileContents with this:

#include <stdio.h>
#include <stdlib.h>
#include <errno.h>

/*
 * requires the following in PATH: sh, touch, cat, nm, gawk, sort.
 * we use CWD for temp files at main() and at the shell script because:
 * - mktemp (shell) is non standard, mkstemps (API) is unavailable on windows.
 * - we choose temp names which don't require quoting in shell or C strings.
 * - a relative path with forward slashes is identical on *nix/windows
 *   (FILE APIs, shell, tcc CLI arguments), but absolute path on windows will
 *   require conversions between shell/API/CLI representations.
 */
#define SCRIPT_PATH "./tmp-tcc104.sh"
#define TMP1        "./tmp-tcc104.1"
#define TMP2        "./tmp-tcc104.2"

int main(int argc, char **argv)
{
    extern char const cfileContents[];
    const char *tcc;
    FILE *f;
    int r;

    /* TESTED_TCC is bin + args, should stay unquoted */
    if (!(tcc = argc > 1 ? argv[1] : getenv("TESTED_TCC")))
        return perror("unknown tcc executable"), 1;

    if (!(f = fopen(SCRIPT_PATH, "wb")))
        return perror("cannot create script"), 1;
    fprintf(f,
            "set -e\n"
            "LC_ALL=C; export LC_ALL\n"

            "tmp1=%s; touch $tmp1\n"
            "tmp2=%s; touch $tmp2\n"

            "cat <<\\CFILE > $tmp1\n"
            "%s\n"
            "CFILE\n"

            "%s -c -xc $tmp1 -o $tmp2\n"
            "nm -Ptx $tmp2 > $tmp1\n"
            "gawk '{ if($2 == \"T\") print $1 }' $tmp1 > $tmp2\n"
            "sort $tmp2\n"
            ,
            TMP1, TMP2, cfileContents, tcc);
    fclose(f);

    r = system("sh " SCRIPT_PATH);
    remove(TMP1);
    remove(TMP2);
    remove(SCRIPT_PATH);

    return r;
}

char const cfileContents[]=
"inline void inline_inline_2decl_only(void);\n"
...


On Wednesday, June 19, 2019 1:49 PM, avih <[hidden email]> wrote:


Peter,

You're correct, but on windows tcc isn't posix compliant, hence no mkstemp.

Additionally, `system(...)` on windows invokes the windows shell, which typically is not the shell where `make` is invoked (and which make uses), and it has different path expectations than the test shell.

As for pipe failure, you could (and I'm currently doing while working on it) use the posix `set -e` and execute the commands individually while still failing on errors. I'm aware that `set -e` can be a can of worms, but for this simple script it doesn't pose hidden pitfalls that I can identify.

As for `mktemp` not being standard, I wasn't aware of that, thanks. For now I'll use it, but I won't post it for review before I have a reasonable alternative if it's not available.

Alternatively, if you have a version which uses a shell script (and the tcc path taken from the TCC env var which the makefile sets anyway) then you can push it instead of me keep working on it.

Avi


On Wednesday, June 19, 2019 1:17 PM, Petr Skočík <[hidden email]> wrote:


The test started out as a shell script. It could have been a pipeline,
but what I dislike about pipelines in POSIX shell is you can't
really error check them well because if a link fails and the last one
succeeds, you get a success.

That is why I wrote I ended up invoking the links from C
using tempfiles as glue. You can theoretically do the same from a shell
but mktemp isn't standard but you can create tempfiles from C standardly
(mkstemps() is POSIX, tmpnam() is ISO C) and the C code isn't much
longer than the shell code.

Petr

On 6/19/19 11:33 AM, avih wrote:

> Hi Christian and thanks for the quick response.
>
> I can confirm that your patch fixes the issue (makes the diff go away).
>
> However, before the patch it definitely invoked the cygwin sort because
> my PATH only has cygwin's /bin and /local/bin (translated to correct
> windows paths when test 104 executes) and prefixed with TCC's root path.
> I tested this by printing getenv("PATH") at 104_inline_test.c .
>
> I can also confirm the issue simply from cygwin's shell that `sort`
> changes the order of 104_inline_test.expect:
> $ cat tests/tests2/104_inline_test.expect | sort # <-- inst_... is
> before inst2_... i.e. MODIFIED.
>
> while
>
> $ cat tests/tests2/104_inline_test.expect | LC_ALL=C sort # <-- inst2...
> is before inst_...  i.e. unmodified = OK
>
> Alternative to your patch, adding `export LC_ALL = C` at the beginning
> of tests/tests2/Makefile also fixes the issue for me.
>
> Running `locale` at the shell prints this for me:
> $ locale
> LANG=en_US.UTF-8
> LC_CTYPE="en_US.UTF-8"
> LC_NUMERIC="en_US.UTF-8"
> LC_TIME="en_US.UTF-8"
> LC_COLLATE="en_US.UTF-8"
> LC_MONETARY="en_US.UTF-8"
> LC_MESSAGES="en_US.UTF-8"
> LC_ALL=
>
> Please don't push this patch to mob, because it adds another potential
> complexity by mixing unix/windows tools.
>
> I'll try to modify 104_inline_test.c such that it uses only one system
> invocation of shell -c "<commands>" so that it effectively becomes a
> shell script, and post the result of my attempt here to get some feedback.
>
> Thanks again,
> Avi
>
>
> On Wednesday, June 19, 2019 8:20 AM, Christian Jullien
> <[hidden email]> wrote:
>
>
> Avih,
> I quickly hacked 104 test to start to make it work but I’m not the
> author of this test. I let Petr improve it.
> I fully agree with you that mixing C code and calling system (which
> forks a cmd.exe on Windows) to finally execute gnu commands that require
> Cygwin is a BAD idea!!
> As you said, it’s much easier if 104 test generates a .o which the whole
> ‘unix’ infrastructure test will check. I let the maintainer of this test
> decide what to do.
>  
> About the diff, I don’t understand quite well what happens. Among other,
> it calls system(“sort …”); which spawns cmd.exe and then execute first
> sort seen in path. Depending on your PATH it can be Windows sort (if
> Windows path comes first) or Cygwin version if this one comes first.
>  
> Can you please try this diff which works as good for me but now forces
> sort Windows version:
>  
> diff --git a/tests/tests2/104_inline_test.c b/tests/tests2/104_inline_test.c
> index cb288d2..d191602 100644
> --- a/tests/tests2/104_inline_test.c
> +++ b/tests/tests2/104_inline_test.c
> @@ -5,8 +5,8 @@
>  
> #if __linux__ || __APPLE__
> #define SYS_WHICH_NM "which nm >/dev/null 2>&1"
> +#define SYS_SORT     "sort"
> #define TCC_COMPILER "../../tcc"
> -#define SYS_AWK
>  
> char c[]="/tmp/tcc-XXXXXX"; char o[]="/tmp/tcc-XXXXXX";
> static int mktempfile(char *buf)
> @@ -15,6 +15,7 @@ static int mktempfile(char *buf)
> }
> #elif defined(_WIN32)
> #define SYS_WHICH_NM  "which nm >nul 2>&1"
> +#define SYS_SORT      "%systemroot%\\system32\\sort /LOCALE C"
>  
> #if defined(_WIN64)
> #define TCC_COMPILER "..\\..\\win32\\x86_64-win32-tcc"
> @@ -72,7 +73,7 @@ int main(int C, char **V)
>          sprintf(buf, "%s -c -xc %s -o %s", V[1]?V[1]:TCC_COMPILER, c,
> o); if(0!=system(buf)){ r=1;goto out;}
>          sprintf(buf, "nm -Ptx %s > %s", o, c); if(system(buf))
> {r=1;goto out;}
>          sprintf(buf, "gawk '{ if($2 == \"T\") print $1 }' %s > %s", c,
> o); if(system(buf)) {r=1;goto out;}
> -        sprintf(buf, "sort %s", o); if(system(buf)) {r=1;goto out;}
> +        sprintf(buf, "%s %s", SYS_SORT, o); if(system(buf)) {r=1;goto out;}
> out:
>         remove(c);
>         remove(o);
>  
> *From:*Tinycc-devel
> [mailto:tinycc-devel-bounces+eligis=[hidden email]] *On Behalf Of
> *avih
> *Sent:* Tuesday, June 18, 2019 21:01
> *To:* [hidden email]; [hidden email]; 'Michael Matz'
> *Subject:* Re: [Tinycc-devel] test 104 fails on windows: missing mkstemps
>  
> Well, the diffs are not really diffs. They just moved. Looks like `sort`
> didn't work as expected, or maybe it's some locale issue (mine is
> en_US.UTF-8 at cygwin, and en-US at windows).
>  
> A script should handle this too, possibly with LC_ALL=C (and make sure
> the reference file is generated with the same sort locale).
>  
> If someone could split it to program+script then I can test the test in
> various use cases and make adjustment if required (I'm terrible with
> Makefiles but reasonably useful with shell).
>  
> On Tuesday, June 18, 2019 9:50 PM, avih <[hidden email]> wrote:
>  
> After commit d39c49db  Remove empty conditional _WIN32 code
>
>
> and some hacking of the code (it's an unhealthy mix of basically running
> a shell script from a program compiled using tcc for windows), I get the
> following 2 diffs:
>
>
> +inst_extern_inline_postdeclared
> +inst_extern_inline_predeclared
>
>
>
> and
>
>
> -inst_extern_inline_postdeclared
> -inst_extern_inline_predeclared
>
>
>
> I'm running it in a cygwin environment and the tools (nm, sort, gawk)
> are cygwin tools, while the tested tcc is normal mingw tcc for windows
> (which I build in a reproducible way using my script).
>
>
>
> Regardless of these two diffs, I think the test should be composed of a
> program and a normal shell script (which uses mktemp, awk, sort etc), so
> that the paths are consistent between the tools.
>
>
> Also, the TCC path is hardcoded at the test, but in fact it's parametric
> at the makefile as $(TCC), so it's better to use that instead (but then
> there are forward/backward slash issues which need to be handled too,
> because system(...) in win32 expects backward slashes, but $(TCC) at the
> makefile has forward slashes). Making this a program + a script should
> implicitly solve this issue as well.
>
>
>
> After all, a working shell+tools is assumed for this test anyway, but
> the current way of using system(...) from a win32 program (compiled
> using tcc for windows) invokes a windows shell which can be inconsistent
> with the actual shell where `make` runs.
>
>
> Avi
>
>
>
>
>
>  
> On Tuesday, June 18, 2019 12:11 AM, avih <[hidden email]> wrote:
>  
> Hmm.. I now see that test 104 uses signal and nm, so it might require
> some effort to make it work on windows.
>  
> Nevertheless, considering the recent breakage specifically on windows
> from related code, I think it would be beneficial if this test could be
> made to work on windows,
>  
> On Monday, June 17, 2019 11:54 PM, avih <[hidden email]> wrote:
>  
> Wouldn't it be better to just create a known/fixed file instead?
> (assuming the test doesn't need explicitly mkstemps, I didn't look at
> its actual code).
>  
> On Monday, June 17, 2019 11:50 PM, Christian Jullien <[hidden email]>
> wrote:
>  
> Yes it has been previously reported.
>  
> Michael, as said in a private mail, I agree with you that this test
> should be skipped on Windows.
>  
> C.
>  
> *From:*Tinycc-devel
> [mailto:tinycc-devel-bounces+eligis=[hidden email]] *On Behalf Of
> *avih
> *Sent:* Monday, June 17, 2019 22:46
> *To:* Tinycc-devel Mailing List
> *Subject:* [Tinycc-devel] test 104 fails on windows: missing mkstemps
>  
> Test 104 log on windows (both tcc32 and tcc 64):
>  
> Test: 104_inline_test...
> --- 104_inline_test.expect      2019-06-17 23:42:00.162697100 +0300
> +++ 104_inline_test.output      2019-06-17 23:42:35.531550400 +0300
> @@ -1,25 +1,2 @@
> -extern_extern_postdeclared
> -extern_extern_postdeclared2
> -extern_extern_predeclared
> -extern_extern_predeclared2
> -extern_extern_prepostdeclared
> -extern_extern_prepostdeclared2
> -extern_extern_undeclared
> -extern_extern_undeclared2
> -extern_postdeclared
> -extern_postdeclared2
> -extern_predeclared
> -extern_predeclared2
> -extern_prepostdeclared
> -extern_undeclared
> -extern_undeclared2
> -inst2_extern_inline_postdeclared
> -inst2_extern_inline_predeclared
> -inst3_extern_inline_predeclared
> -inst_extern_inline_postdeclared
> -inst_extern_inline_predeclared
> -main
> -noinst_extern_inline_func
> -noinst_extern_inline_postdeclared
> -noinst_extern_inline_postdeclared2
> -noinst_extern_inline_undeclared
> +104_inline_test.c:30: warning: implicit declaration of function 'mkstemps'
> +tcc: error: undefined symbol 'mkstemps'
> make[1]: *** [Makefile:70: 104_inline_test.test] Error 1
> Test: 105_local_extern...
> make[1]: Target 'all' not remade because of errors.
>  
>  
>  
>  
>  
>
>






_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

avih
As usual, I forgot to attach the patch. Attached now.


On Wednesday, June 19, 2019 7:28 PM, avih <[hidden email]> wrote:


If no objects to the attached patch, then I'll push it to mob.

In a nutshell: it creates and executes a shell script, where the tcc binary path (and arguments) are taken from the Makefile (where it `export TESTED_TCC = $(TCC)`). It was tested on Ubuntu and Windows (cygwin).

While I think it's better than before, it's still rather awkward, and at some stage we'll probably want to compile cFileContents directly from the makefile and then execute the commands at the script also at the Makefile.

The patch replaces everything before cFileContents with this:

#include <stdio.h>
#include <stdlib.h>
#include <errno.h>

/*
 * requires the following in PATH: sh, touch, cat, nm, gawk, sort.
 * we use CWD for temp files at main() and at the shell script because:
 * - mktemp (shell) is non standard, mkstemps (API) is unavailable on windows.
 * - we choose temp names which don't require quoting in shell or C strings.
 * - a relative path with forward slashes is identical on *nix/windows
 *   (FILE APIs, shell, tcc CLI arguments), but absolute path on windows will
 *   require conversions between shell/API/CLI representations.
 */
#define SCRIPT_PATH "./tmp-tcc104.sh"
#define TMP1        "./tmp-tcc104.1"
#define TMP2        "./tmp-tcc104.2"

int main(int argc, char **argv)
{
    extern char const cfileContents[];
    const char *tcc;
    FILE *f;
    int r;

    /* TESTED_TCC is bin + args, should stay unquoted */
    if (!(tcc = argc > 1 ? argv[1] : getenv("TESTED_TCC")))
        return perror("unknown tcc executable"), 1;

    if (!(f = fopen(SCRIPT_PATH, "wb")))
        return perror("cannot create script"), 1;
    fprintf(f,
            "set -e\n"
            "LC_ALL=C; export LC_ALL\n"

            "tmp1=%s; touch $tmp1\n"
            "tmp2=%s; touch $tmp2\n"

            "cat <<\\CFILE > $tmp1\n"
            "%s\n"
            "CFILE\n"

            "%s -c -xc $tmp1 -o $tmp2\n"
            "nm -Ptx $tmp2 > $tmp1\n"
            "gawk '{ if($2 == \"T\") print $1 }' $tmp1 > $tmp2\n"
            "sort $tmp2\n"
            ,
            TMP1, TMP2, cfileContents, tcc);
    fclose(f);

    r = system("sh " SCRIPT_PATH);
    remove(TMP1);
    remove(TMP2);
    remove(SCRIPT_PATH);

    return r;
}

char const cfileContents[]=
"inline void inline_inline_2decl_only(void);\n"
...


On Wednesday, June 19, 2019 1:49 PM, avih <[hidden email]> wrote:


Peter,

You're correct, but on windows tcc isn't posix compliant, hence no mkstemp.

Additionally, `system(...)` on windows invokes the windows shell, which typically is not the shell where `make` is invoked (and which make uses), and it has different path expectations than the test shell.

As for pipe failure, you could (and I'm currently doing while working on it) use the posix `set -e` and execute the commands individually while still failing on errors. I'm aware that `set -e` can be a can of worms, but for this simple script it doesn't pose hidden pitfalls that I can identify.

As for `mktemp` not being standard, I wasn't aware of that, thanks. For now I'll use it, but I won't post it for review before I have a reasonable alternative if it's not available.

Alternatively, if you have a version which uses a shell script (and the tcc path taken from the TCC env var which the makefile sets anyway) then you can push it instead of me keep working on it.

Avi


On Wednesday, June 19, 2019 1:17 PM, Petr Skočík <[hidden email]> wrote:


The test started out as a shell script. It could have been a pipeline,
but what I dislike about pipelines in POSIX shell is you can't
really error check them well because if a link fails and the last one
succeeds, you get a success.

That is why I wrote I ended up invoking the links from C
using tempfiles as glue. You can theoretically do the same from a shell
but mktemp isn't standard but you can create tempfiles from C standardly
(mkstemps() is POSIX, tmpnam() is ISO C) and the C code isn't much
longer than the shell code.

Petr

On 6/19/19 11:33 AM, avih wrote:

> Hi Christian and thanks for the quick response.
>
> I can confirm that your patch fixes the issue (makes the diff go away).
>
> However, before the patch it definitely invoked the cygwin sort because
> my PATH only has cygwin's /bin and /local/bin (translated to correct
> windows paths when test 104 executes) and prefixed with TCC's root path.
> I tested this by printing getenv("PATH") at 104_inline_test.c .
>
> I can also confirm the issue simply from cygwin's shell that `sort`
> changes the order of 104_inline_test.expect:
> $ cat tests/tests2/104_inline_test.expect | sort # <-- inst_... is
> before inst2_... i.e. MODIFIED.
>
> while
>
> $ cat tests/tests2/104_inline_test.expect | LC_ALL=C sort # <-- inst2...
> is before inst_...  i.e. unmodified = OK
>
> Alternative to your patch, adding `export LC_ALL = C` at the beginning
> of tests/tests2/Makefile also fixes the issue for me.
>
> Running `locale` at the shell prints this for me:
> $ locale
> LANG=en_US.UTF-8
> LC_CTYPE="en_US.UTF-8"
> LC_NUMERIC="en_US.UTF-8"
> LC_TIME="en_US.UTF-8"
> LC_COLLATE="en_US.UTF-8"
> LC_MONETARY="en_US.UTF-8"
> LC_MESSAGES="en_US.UTF-8"
> LC_ALL=
>
> Please don't push this patch to mob, because it adds another potential
> complexity by mixing unix/windows tools.
>
> I'll try to modify 104_inline_test.c such that it uses only one system
> invocation of shell -c "<commands>" so that it effectively becomes a
> shell script, and post the result of my attempt here to get some feedback.
>
> Thanks again,
> Avi
>
>
> On Wednesday, June 19, 2019 8:20 AM, Christian Jullien
> <[hidden email]> wrote:
>
>
> Avih,
> I quickly hacked 104 test to start to make it work but I’m not the
> author of this test. I let Petr improve it.
> I fully agree with you that mixing C code and calling system (which
> forks a cmd.exe on Windows) to finally execute gnu commands that require
> Cygwin is a BAD idea!!
> As you said, it’s much easier if 104 test generates a .o which the whole
> ‘unix’ infrastructure test will check. I let the maintainer of this test
> decide what to do.
>  
> About the diff, I don’t understand quite well what happens. Among other,
> it calls system(“sort …”); which spawns cmd.exe and then execute first
> sort seen in path. Depending on your PATH it can be Windows sort (if
> Windows path comes first) or Cygwin version if this one comes first.
>  
> Can you please try this diff which works as good for me but now forces
> sort Windows version:
>  
> diff --git a/tests/tests2/104_inline_test.c b/tests/tests2/104_inline_test.c
> index cb288d2..d191602 100644
> --- a/tests/tests2/104_inline_test.c
> +++ b/tests/tests2/104_inline_test.c
> @@ -5,8 +5,8 @@
>  
> #if __linux__ || __APPLE__
> #define SYS_WHICH_NM "which nm >/dev/null 2>&1"
> +#define SYS_SORT     "sort"
> #define TCC_COMPILER "../../tcc"
> -#define SYS_AWK
>  
> char c[]="/tmp/tcc-XXXXXX"; char o[]="/tmp/tcc-XXXXXX";
> static int mktempfile(char *buf)
> @@ -15,6 +15,7 @@ static int mktempfile(char *buf)
> }
> #elif defined(_WIN32)
> #define SYS_WHICH_NM  "which nm >nul 2>&1"
> +#define SYS_SORT      "%systemroot%\\system32\\sort /LOCALE C"
>  
> #if defined(_WIN64)
> #define TCC_COMPILER "..\\..\\win32\\x86_64-win32-tcc"
> @@ -72,7 +73,7 @@ int main(int C, char **V)
>          sprintf(buf, "%s -c -xc %s -o %s", V[1]?V[1]:TCC_COMPILER, c,
> o); if(0!=system(buf)){ r=1;goto out;}
>          sprintf(buf, "nm -Ptx %s > %s", o, c); if(system(buf))
> {r=1;goto out;}
>          sprintf(buf, "gawk '{ if($2 == \"T\") print $1 }' %s > %s", c,
> o); if(system(buf)) {r=1;goto out;}
> -        sprintf(buf, "sort %s", o); if(system(buf)) {r=1;goto out;}
> +        sprintf(buf, "%s %s", SYS_SORT, o); if(system(buf)) {r=1;goto out;}
> out:
>         remove(c);
>         remove(o);
>  
> *From:*Tinycc-devel
> [mailto:tinycc-devel-bounces+eligis=[hidden email]] *On Behalf Of
> *avih
> *Sent:* Tuesday, June 18, 2019 21:01
> *To:* [hidden email]; [hidden email]; 'Michael Matz'
> *Subject:* Re: [Tinycc-devel] test 104 fails on windows: missing mkstemps
>  
> Well, the diffs are not really diffs. They just moved. Looks like `sort`
> didn't work as expected, or maybe it's some locale issue (mine is
> en_US.UTF-8 at cygwin, and en-US at windows).
>  
> A script should handle this too, possibly with LC_ALL=C (and make sure
> the reference file is generated with the same sort locale).
>  
> If someone could split it to program+script then I can test the test in
> various use cases and make adjustment if required (I'm terrible with
> Makefiles but reasonably useful with shell).
>  
> On Tuesday, June 18, 2019 9:50 PM, avih <[hidden email]> wrote:
>  
> After commit d39c49db  Remove empty conditional _WIN32 code
>
>
> and some hacking of the code (it's an unhealthy mix of basically running
> a shell script from a program compiled using tcc for windows), I get the
> following 2 diffs:
>
>
> +inst_extern_inline_postdeclared
> +inst_extern_inline_predeclared
>
>
>
> and
>
>
> -inst_extern_inline_postdeclared
> -inst_extern_inline_predeclared
>
>
>
> I'm running it in a cygwin environment and the tools (nm, sort, gawk)
> are cygwin tools, while the tested tcc is normal mingw tcc for windows
> (which I build in a reproducible way using my script).
>
>
>
> Regardless of these two diffs, I think the test should be composed of a
> program and a normal shell script (which uses mktemp, awk, sort etc), so
> that the paths are consistent between the tools.
>
>
> Also, the TCC path is hardcoded at the test, but in fact it's parametric
> at the makefile as $(TCC), so it's better to use that instead (but then
> there are forward/backward slash issues which need to be handled too,
> because system(...) in win32 expects backward slashes, but $(TCC) at the
> makefile has forward slashes). Making this a program + a script should
> implicitly solve this issue as well.
>
>
>
> After all, a working shell+tools is assumed for this test anyway, but
> the current way of using system(...) from a win32 program (compiled
> using tcc for windows) invokes a windows shell which can be inconsistent
> with the actual shell where `make` runs.
>
>
> Avi
>
>
>
>
>
>  
> On Tuesday, June 18, 2019 12:11 AM, avih <[hidden email]> wrote:
>  
> Hmm.. I now see that test 104 uses signal and nm, so it might require
> some effort to make it work on windows.
>  
> Nevertheless, considering the recent breakage specifically on windows
> from related code, I think it would be beneficial if this test could be
> made to work on windows,
>  
> On Monday, June 17, 2019 11:54 PM, avih <[hidden email]> wrote:
>  
> Wouldn't it be better to just create a known/fixed file instead?
> (assuming the test doesn't need explicitly mkstemps, I didn't look at
> its actual code).
>  
> On Monday, June 17, 2019 11:50 PM, Christian Jullien <[hidden email]>
> wrote:
>  
> Yes it has been previously reported.
>  
> Michael, as said in a private mail, I agree with you that this test
> should be skipped on Windows.
>  
> C.
>  
> *From:*Tinycc-devel
> [mailto:tinycc-devel-bounces+eligis=[hidden email]] *On Behalf Of
> *avih
> *Sent:* Monday, June 17, 2019 22:46
> *To:* Tinycc-devel Mailing List
> *Subject:* [Tinycc-devel] test 104 fails on windows: missing mkstemps
>  
> Test 104 log on windows (both tcc32 and tcc 64):
>  
> Test: 104_inline_test...
> --- 104_inline_test.expect      2019-06-17 23:42:00.162697100 +0300
> +++ 104_inline_test.output      2019-06-17 23:42:35.531550400 +0300
> @@ -1,25 +1,2 @@
> -extern_extern_postdeclared
> -extern_extern_postdeclared2
> -extern_extern_predeclared
> -extern_extern_predeclared2
> -extern_extern_prepostdeclared
> -extern_extern_prepostdeclared2
> -extern_extern_undeclared
> -extern_extern_undeclared2
> -extern_postdeclared
> -extern_postdeclared2
> -extern_predeclared
> -extern_predeclared2
> -extern_prepostdeclared
> -extern_undeclared
> -extern_undeclared2
> -inst2_extern_inline_postdeclared
> -inst2_extern_inline_predeclared
> -inst3_extern_inline_predeclared
> -inst_extern_inline_postdeclared
> -inst_extern_inline_predeclared
> -main
> -noinst_extern_inline_func
> -noinst_extern_inline_postdeclared
> -noinst_extern_inline_postdeclared2
> -noinst_extern_inline_undeclared
> +104_inline_test.c:30: warning: implicit declaration of function 'mkstemps'
> +tcc: error: undefined symbol 'mkstemps'
> make[1]: *** [Makefile:70: 104_inline_test.test] Error 1
> Test: 105_local_extern...
> make[1]: Target 'all' not remade because of errors.
>  
>  
>  
>  
>  
>
>








_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

0001-test-104-simplify-parametrize-more-robust-on-windows.patch (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

Christian Jullien-3

As long as it works on Windows, it’s Ok for me.

When pushed on mod I’ll adapt my companion win32/Makefile to pass TESTED_TCC as suggested.

 

C.

 

 

From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=[hidden email]] On Behalf Of avih
Sent: Wednesday, June 19, 2019 18:33
To: Petr Skočík; [hidden email]
Subject: Re: [Tinycc-devel] test 104 fails on windows: missing mkstemps

 

As usual, I forgot to attach the patch. Attached now.

 

On Wednesday, June 19, 2019 7:28 PM, avih <[hidden email]> wrote:

 

If no objects to the attached patch, then I'll push it to mob.

 

In a nutshell: it creates and executes a shell script, where the tcc binary path (and arguments) are taken from the Makefile (where it `export TESTED_TCC = $(TCC)`). It was tested on Ubuntu and Windows (cygwin).



While I think it's better than before, it's still rather awkward, and at some stage we'll probably want to compile cFileContents directly from the makefile and then execute the commands at the script also at the Makefile.

 

The patch replaces everything before cFileContents with this:

 

#include <stdio.h>
#include <stdlib.h>
#include <errno.h>

/*
 * requires the following in PATH: sh, touch, cat, nm, gawk, sort.
 * we use CWD for temp files at main() and at the shell script because:
 * - mktemp (shell) is non standard, mkstemps (API) is unavailable on windows.
 * - we choose temp names which don't require quoting in shell or C strings.
 * - a relative path with forward slashes is identical on *nix/windows
 *   (FILE APIs, shell, tcc CLI arguments), but absolute path on windows will
 *   require conversions between shell/API/CLI representations.
 */
#define SCRIPT_PATH "./tmp-tcc104.sh"
#define TMP1        "./tmp-tcc104.1"
#define TMP2        "./tmp-tcc104.2"

int main(int argc, char **argv)
{
    extern char const cfileContents[];
    const char *tcc;
    FILE *f;
    int r;

    /* TESTED_TCC is bin + args, should stay unquoted */
    if (!(tcc = argc > 1 ? argv[1] : getenv("TESTED_TCC")))
        return perror("unknown tcc executable"), 1;

    if (!(f = fopen(SCRIPT_PATH, "wb")))
        return perror("cannot create script"), 1;
    fprintf(f,
            "set -e\n"
            "LC_ALL=C; export LC_ALL\n"

            "tmp1=%s; touch $tmp1\n"
            "tmp2=%s; touch $tmp2\n"

            "cat <<\\CFILE > $tmp1\n"
            "%s\n"
            "CFILE\n"

            "%s -c -xc $tmp1 -o $tmp2\n"
            "nm -Ptx $tmp2 > $tmp1\n"
            "gawk '{ if($2 == \"T\") print $1 }' $tmp1 > $tmp2\n"
            "sort $tmp2\n"
            ,
            TMP1, TMP2, cfileContents, tcc);
    fclose(f);

    r = system("sh " SCRIPT_PATH);
    remove(TMP1);
    remove(TMP2);
    remove(SCRIPT_PATH);

    return r;
}



char const cfileContents[]=
"inline void inline_inline_2decl_only(void);\n"
...

 

On Wednesday, June 19, 2019 1:49 PM, avih <[hidden email]> wrote:

 

Peter,

 

You're correct, but on windows tcc isn't posix compliant, hence no mkstemp.

 

Additionally, `system(...)` on windows invokes the windows shell, which typically is not the shell where `make` is invoked (and which make uses), and it has different path expectations than the test shell.

 

As for pipe failure, you could (and I'm currently doing while working on it) use the posix `set -e` and execute the commands individually while still failing on errors. I'm aware that `set -e` can be a can of worms, but for this simple script it doesn't pose hidden pitfalls that I can identify.

 

As for `mktemp` not being standard, I wasn't aware of that, thanks. For now I'll use it, but I won't post it for review before I have a reasonable alternative if it's not available.

 

Alternatively, if you have a version which uses a shell script (and the tcc path taken from the TCC env var which the makefile sets anyway) then you can push it instead of me keep working on it.

 

Avi

 

On Wednesday, June 19, 2019 1:17 PM, Petr Skočík <[hidden email]> wrote:

 

The test started out as a shell script. It could have been a pipeline,
but what I dislike about pipelines in POSIX shell is you can't
really error check them well because if a link fails and the last one
succeeds, you get a success.

That is why I wrote I ended up invoking the links from C
using tempfiles as glue. You can theoretically do the same from a shell
but mktemp isn't standard but you can create tempfiles from C standardly
(mkstemps() is POSIX, tmpnam() is ISO C) and the C code isn't much
longer than the shell code.

Petr


On 6/19/19 11:33 AM, avih wrote:


> Hi Christian and thanks for the quick response.
>
> I can confirm that your patch fixes the issue (makes the diff go away).
>
> However, before the patch it definitely invoked the cygwin sort because
> my PATH only has cygwin's /bin and /local/bin (translated to correct
> windows paths when test 104 executes) and prefixed with TCC's root path.
> I tested this by printing getenv("PATH") at 104_inline_test.c .
>
> I can also confirm the issue simply from cygwin's shell that `sort`
> changes the order of 104_inline_test.expect:
> $ cat tests/tests2/104_inline_test.expect | sort # <-- inst_... is
> before inst2_... i.e. MODIFIED.
>
> while
>
> $ cat tests/tests2/104_inline_test.expect | LC_ALL=C sort # <-- inst2...
> is before inst_...  i.e. unmodified = OK
>
> Alternative to your patch, adding `export LC_ALL = C` at the beginning
> of tests/tests2/Makefile also fixes the issue for me.
>
> Running `locale` at the shell prints this for me:
> $ locale
> LANG=en_US.UTF-8
> LC_CTYPE="en_US.UTF-8"
> LC_NUMERIC="en_US.UTF-8"
> LC_TIME="en_US.UTF-8"
> LC_COLLATE="en_US.UTF-8"
> LC_MONETARY="en_US.UTF-8"
> LC_MESSAGES="en_US.UTF-8"
> LC_ALL=
>
> Please don't push this patch to mob, because it adds another potential
> complexity by mixing unix/windows tools.
>
> I'll try to modify 104_inline_test.c such that it uses only one system
> invocation of shell -c "<commands>" so that it effectively becomes a
> shell script, and post the result of my attempt here to get some feedback.
>
> Thanks again,
> Avi
>
>
> On Wednesday, June 19, 2019 8:20 AM, Christian Jullien
> <[hidden email]> wrote:
>
>
> Avih,
> I quickly hacked 104 test to start to make it work but I’m not the
> author of this test. I let Petr improve it.
> I fully agree with you that mixing C code and calling system (which
> forks a cmd.exe on Windows) to finally execute gnu commands that require
> Cygwin is a BAD idea!!
> As you said, it’s much easier if 104 test generates a .o which the whole
> ‘unix’ infrastructure test will check. I let the maintainer of this test
> decide what to do.
>  
> About the diff, I don’t understand quite well what happens. Among other,
> it calls system(“sort …”); which spawns cmd.exe and then execute first
> sort seen in path. Depending on your PATH it can be Windows sort (if
> Windows path comes first) or Cygwin version if this one comes first.
>  
> Can you please try this diff which works as good for me but now forces
> sort Windows version:
>  
> diff --git a/tests/tests2/104_inline_test.c b/tests/tests2/104_inline_test.c
> index cb288d2..d191602 100644
> --- a/tests/tests2/104_inline_test.c
> +++ b/tests/tests2/104_inline_test.c
> @@ -5,8 +5,8 @@
>  
> #if __linux__ || __APPLE__
> #define SYS_WHICH_NM "which nm >/dev/null 2>&1"
> +#define SYS_SORT     "sort"
> #define TCC_COMPILER "../../tcc"
> -#define SYS_AWK
>  
> char c[]="/tmp/tcc-XXXXXX"; char o[]="/tmp/tcc-XXXXXX";
> static int mktempfile(char *buf)
> @@ -15,6 +15,7 @@ static int mktempfile(char *buf)
> }
> #elif defined(_WIN32)
> #define SYS_WHICH_NM  "which nm >nul 2>&1"
> +#define SYS_SORT      "%systemroot%\\system32\\sort /LOCALE C"
>  
> #if defined(_WIN64)
> #define TCC_COMPILER "..\\..\\win32\\x86_64-win32-tcc"
> @@ -72,7 +73,7 @@ int main(int C, char **V)
>          sprintf(buf, "%s -c -xc %s -o %s", V[1]?V[1]:TCC_COMPILER, c,
> o); if(0!=system(buf)){ r=1;goto out;}
>          sprintf(buf, "nm -Ptx %s > %s", o, c); if(system(buf))
> {r=1;goto out;}
>          sprintf(buf, "gawk '{ if($2 == \"T\") print $1 }' %s > %s", c,
> o); if(system(buf)) {r=1;goto out;}
> -        sprintf(buf, "sort %s", o); if(system(buf)) {r=1;goto out;}
> +        sprintf(buf, "%s %s", SYS_SORT, o); if(system(buf)) {r=1;goto out;}
> out:
>         remove(c);
>         remove(o);
>  
> *From:*Tinycc-devel
> [mailto:tinycc-devel-bounces+eligis=[hidden email]] *On Behalf Of
> *avih
> *Sent:* Tuesday, June 18, 2019 21:01
> *To:* [hidden email]; [hidden email]; 'Michael Matz'
> *Subject:* Re: [Tinycc-devel] test 104 fails on windows: missing mkstemps
>  
> Well, the diffs are not really diffs. They just moved. Looks like `sort`
> didn't work as expected, or maybe it's some locale issue (mine is
> en_US.UTF-8 at cygwin, and en-US at windows).
>  
> A script should handle this too, possibly with LC_ALL=C (and make sure
> the reference file is generated with the same sort locale).
>  
> If someone could split it to program+script then I can test the test in
> various use cases and make adjustment if required (I'm terrible with
> Makefiles but reasonably useful with shell).
>  
> On Tuesday, June 18, 2019 9:50 PM, avih <[hidden email]> wrote:
>  
> After commit d39c49db  Remove empty conditional _WIN32 code
>
>
> and some hacking of the code (it's an unhealthy mix of basically running
> a shell script from a program compiled using tcc for windows), I get the
> following 2 diffs:
>
>
> +inst_extern_inline_postdeclared
> +inst_extern_inline_predeclared
>
>
>
> and
>
>
> -inst_extern_inline_postdeclared
> -inst_extern_inline_predeclared
>
>
>
> I'm running it in a cygwin environment and the tools (nm, sort, gawk)
> are cygwin tools, while the tested tcc is normal mingw tcc for windows
> (which I build in a reproducible way using my script).
>
>
>
> Regardless of these two diffs, I think the test should be composed of a
> program and a normal shell script (which uses mktemp, awk, sort etc), so
> that the paths are consistent between the tools.
>
>
> Also, the TCC path is hardcoded at the test, but in fact it's parametric
> at the makefile as $(TCC), so it's better to use that instead (but then
> there are forward/backward slash issues which need to be handled too,
> because system(...) in win32 expects backward slashes, but $(TCC) at the
> makefile has forward slashes). Making this a program + a script should
> implicitly solve this issue as well.
>
>
>
> After all, a working shell+tools is assumed for this test anyway, but
> the current way of using system(...) from a win32 program (compiled
> using tcc for windows) invokes a windows shell which can be inconsistent
> with the actual shell where `make` runs.
>
>
> Avi
>
>
>
>
>
>  
> On Tuesday, June 18, 2019 12:11 AM, avih <[hidden email]> wrote:
>  
> Hmm.. I now see that test 104 uses signal and nm, so it might require
> some effort to make it work on windows.
>  
> Nevertheless, considering the recent breakage specifically on windows
> from related code, I think it would be beneficial if this test could be
> made to work on windows,
>  
> On Monday, June 17, 2019 11:54 PM, avih <[hidden email]> wrote:
>  
> Wouldn't it be better to just create a known/fixed file instead?
> (assuming the test doesn't need explicitly mkstemps, I didn't look at
> its actual code).
>  
> On Monday, June 17, 2019 11:50 PM, Christian Jullien <[hidden email]>
> wrote:
>  
> Yes it has been previously reported.
>  
> Michael, as said in a private mail, I agree with you that this test
> should be skipped on Windows.
>  
> C.
>  
> *From:*Tinycc-devel
> [mailto:tinycc-devel-bounces+eligis=[hidden email]] *On Behalf Of
> *avih
> *Sent:* Monday, June 17, 2019 22:46
> *To:* Tinycc-devel Mailing List
> *Subject:* [Tinycc-devel] test 104 fails on windows: missing mkstemps
>  
> Test 104 log on windows (both tcc32 and tcc 64):
>  
> Test: 104_inline_test...
> --- 104_inline_test.expect      2019-06-17 23:42:00.162697100 +0300
> +++ 104_inline_test.output      2019-06-17 23:42:35.531550400 +0300
> @@ -1,25 +1,2 @@
> -extern_extern_postdeclared
> -extern_extern_postdeclared2
> -extern_extern_predeclared
> -extern_extern_predeclared2
> -extern_extern_prepostdeclared
> -extern_extern_prepostdeclared2
> -extern_extern_undeclared
> -extern_extern_undeclared2
> -extern_postdeclared
> -extern_postdeclared2
> -extern_predeclared
> -extern_predeclared2
> -extern_prepostdeclared
> -extern_undeclared
> -extern_undeclared2
> -inst2_extern_inline_postdeclared
> -inst2_extern_inline_predeclared
> -inst3_extern_inline_predeclared
> -inst_extern_inline_postdeclared
> -inst_extern_inline_predeclared
> -main
> -noinst_extern_inline_func
> -noinst_extern_inline_postdeclared
> -noinst_extern_inline_postdeclared2
> -noinst_extern_inline_undeclared
> +104_inline_test.c:30: warning: implicit declaration of function 'mkstemps'
> +tcc: error: undefined symbol 'mkstemps'
> make[1]: *** [Makefile:70: 104_inline_test.test] Error 1
> Test: 105_local_extern...
> make[1]: Target 'all' not remade because of errors.
>  
>  
>  
>  
>  
>
>

 

 

 

_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

avih
Pushed a slightly simpler version. Also, changed gawk to awk for wider compatibility.


On Wednesday, June 19, 2019, 8:45:30 PM GMT+3, Christian Jullien <[hidden email]> wrote:


As long as it works on Windows, it’s Ok for me.

When pushed on mod I’ll adapt my companion win32/Makefile to pass TESTED_TCC as suggested.

 

C.

 

 

From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=[hidden email]] On Behalf Of avih
Sent: Wednesday, June 19, 2019 18:33
To: Petr Skočík; [hidden email]
Subject: Re: [Tinycc-devel] test 104 fails on windows: missing mkstemps

 

As usual, I forgot to attach the patch. Attached now.

 

On Wednesday, June 19, 2019 7:28 PM, avih <[hidden email]> wrote:

 

If no objects to the attached patch, then I'll push it to mob.

 

In a nutshell: it creates and executes a shell script, where the tcc binary path (and arguments) are taken from the Makefile (where it `export TESTED_TCC = $(TCC)`). It was tested on Ubuntu and Windows (cygwin).



While I think it's better than before, it's still rather awkward, and at some stage we'll probably want to compile cFileContents directly from the makefile and then execute the commands at the script also at the Makefile.

 

The patch replaces everything before cFileContents with this:

 

#include <stdio.h>
#include <stdlib.h>
#include <errno.h>

/*
 * requires the following in PATH: sh, touch, cat, nm, gawk, sort.
 * we use CWD for temp files at main() and at the shell script because:
 * - mktemp (shell) is non standard, mkstemps (API) is unavailable on windows.
 * - we choose temp names which don't require quoting in shell or C strings.
 * - a relative path with forward slashes is identical on *nix/windows
 *   (FILE APIs, shell, tcc CLI arguments), but absolute path on windows will
 *   require conversions between shell/API/CLI representations.
 */
#define SCRIPT_PATH "./tmp-tcc104.sh"
#define TMP1        "./tmp-tcc104.1"
#define TMP2        "./tmp-tcc104.2"

int main(int argc, char **argv)
{
    extern char const cfileContents[];
    const char *tcc;
    FILE *f;
    int r;

    /* TESTED_TCC is bin + args, should stay unquoted */
    if (!(tcc = argc > 1 ? argv[1] : getenv("TESTED_TCC")))
        return perror("unknown tcc executable"), 1;

    if (!(f = fopen(SCRIPT_PATH, "wb")))
        return perror("cannot create script"), 1;
    fprintf(f,
            "set -e\n"
            "LC_ALL=C; export LC_ALL\n"

            "tmp1=%s; touch $tmp1\n"
            "tmp2=%s; touch $tmp2\n"

            "cat <<\\CFILE > $tmp1\n"
            "%s\n"
            "CFILE\n"

            "%s -c -xc $tmp1 -o $tmp2\n"
            "nm -Ptx $tmp2 > $tmp1\n"
            "gawk '{ if($2 == \"T\") print $1 }' $tmp1 > $tmp2\n"
            "sort $tmp2\n"
            ,
            TMP1, TMP2, cfileContents, tcc);
    fclose(f);

    r = system("sh " SCRIPT_PATH);
    remove(TMP1);
    remove(TMP2);
    remove(SCRIPT_PATH);

    return r;
}



char const cfileContents[]=
"inline void inline_inline_2decl_only(void);\n"
...

 

On Wednesday, June 19, 2019 1:49 PM, avih <[hidden email]> wrote:

 

Peter,

 

You're correct, but on windows tcc isn't posix compliant, hence no mkstemp.

 

Additionally, `system(...)` on windows invokes the windows shell, which typically is not the shell where `make` is invoked (and which make uses), and it has different path expectations than the test shell.

 

As for pipe failure, you could (and I'm currently doing while working on it) use the posix `set -e` and execute the commands individually while still failing on errors. I'm aware that `set -e` can be a can of worms, but for this simple script it doesn't pose hidden pitfalls that I can identify.

 

As for `mktemp` not being standard, I wasn't aware of that, thanks. For now I'll use it, but I won't post it for review before I have a reasonable alternative if it's not available.

 

Alternatively, if you have a version which uses a shell script (and the tcc path taken from the TCC env var which the makefile sets anyway) then you can push it instead of me keep working on it.

 

Avi

 

On Wednesday, June 19, 2019 1:17 PM, Petr Skočík <[hidden email]> wrote:

 

The test started out as a shell script. It could have been a pipeline,
but what I dislike about pipelines in POSIX shell is you can't
really error check them well because if a link fails and the last one
succeeds, you get a success.

That is why I wrote I ended up invoking the links from C
using tempfiles as glue. You can theoretically do the same from a shell
but mktemp isn't standard but you can create tempfiles from C standardly
(mkstemps() is POSIX, tmpnam() is ISO C) and the C code isn't much
longer than the shell code.

Petr


On 6/19/19 11:33 AM, avih wrote:


> Hi Christian and thanks for the quick response.
>
> I can confirm that your patch fixes the issue (makes the diff go away).
>
> However, before the patch it definitely invoked the cygwin sort because
> my PATH only has cygwin's /bin and /local/bin (translated to correct
> windows paths when test 104 executes) and prefixed with TCC's root path.
> I tested this by printing getenv("PATH") at 104_inline_test.c .
>
> I can also confirm the issue simply from cygwin's shell that `sort`
> changes the order of 104_inline_test.expect:
> $ cat tests/tests2/104_inline_test.expect | sort # <-- inst_... is
> before inst2_... i.e. MODIFIED.
>
> while
>
> $ cat tests/tests2/104_inline_test.expect | LC_ALL=C sort # <-- inst2...
> is before inst_...  i.e. unmodified = OK
>
> Alternative to your patch, adding `export LC_ALL = C` at the beginning
> of tests/tests2/Makefile also fixes the issue for me.
>
> Running `locale` at the shell prints this for me:
> $ locale
> LANG=en_US.UTF-8
> LC_CTYPE="en_US.UTF-8"
> LC_NUMERIC="en_US.UTF-8"
> LC_TIME="en_US.UTF-8"
> LC_COLLATE="en_US.UTF-8"
> LC_MONETARY="en_US.UTF-8"
> LC_MESSAGES="en_US.UTF-8"
> LC_ALL=
>
> Please don't push this patch to mob, because it adds another potential
> complexity by mixing unix/windows tools.
>
> I'll try to modify 104_inline_test.c such that it uses only one system
> invocation of shell -c "<commands>" so that it effectively becomes a
> shell script, and post the result of my attempt here to get some feedback.
>
> Thanks again,
> Avi
>
>
> On Wednesday, June 19, 2019 8:20 AM, Christian Jullien
> <[hidden email]> wrote:
>
>
> Avih,
> I quickly hacked 104 test to start to make it work but I’m not the
> author of this test. I let Petr improve it.
> I fully agree with you that mixing C code and calling system (which
> forks a cmd.exe on Windows) to finally execute gnu commands that require
> Cygwin is a BAD idea!!
> As you said, it’s much easier if 104 test generates a .o which the whole
> ‘unix’ infrastructure test will check. I let the maintainer of this test
> decide what to do.
>  
> About the diff, I don’t understand quite well what happens. Among other,
> it calls system(“sort …”); which spawns cmd.exe and then execute first
> sort seen in path. Depending on your PATH it can be Windows sort (if
> Windows path comes first) or Cygwin version if this one comes first.
>  
> Can you please try this diff which works as good for me but now forces
> sort Windows version:
>  
> diff --git a/tests/tests2/104_inline_test.c b/tests/tests2/104_inline_test.c
> index cb288d2..d191602 100644
> --- a/tests/tests2/104_inline_test.c
> +++ b/tests/tests2/104_inline_test.c
> @@ -5,8 +5,8 @@
>  
> #if __linux__ || __APPLE__
> #define SYS_WHICH_NM "which nm >/dev/null 2>&1"
> +#define SYS_SORT     "sort"
> #define TCC_COMPILER "../../tcc"
> -#define SYS_AWK
>  
> char c[]="/tmp/tcc-XXXXXX"; char o[]="/tmp/tcc-XXXXXX";
> static int mktempfile(char *buf)
> @@ -15,6 +15,7 @@ static int mktempfile(char *buf)
> }
> #elif defined(_WIN32)
> #define SYS_WHICH_NM  "which nm >nul 2>&1"
> +#define SYS_SORT      "%systemroot%\\system32\\sort /LOCALE C"
>  
> #if defined(_WIN64)
> #define TCC_COMPILER "..\\..\\win32\\x86_64-win32-tcc"
> @@ -72,7 +73,7 @@ int main(int C, char **V)
>          sprintf(buf, "%s -c -xc %s -o %s", V[1]?V[1]:TCC_COMPILER, c,
> o); if(0!=system(buf)){ r=1;goto out;}
>          sprintf(buf, "nm -Ptx %s > %s", o, c); if(system(buf))
> {r=1;goto out;}
>          sprintf(buf, "gawk '{ if($2 == \"T\") print $1 }' %s > %s", c,
> o); if(system(buf)) {r=1;goto out;}
> -        sprintf(buf, "sort %s", o); if(system(buf)) {r=1;goto out;}
> +        sprintf(buf, "%s %s", SYS_SORT, o); if(system(buf)) {r=1;goto out;}
> out:
>         remove(c);
>         remove(o);
>  
> *From:*Tinycc-devel
> [mailto:tinycc-devel-bounces+eligis=[hidden email]] *On Behalf Of
> *avih
> *Sent:* Tuesday, June 18, 2019 21:01
> *To:* [hidden email]; [hidden email]; 'Michael Matz'
> *Subject:* Re: [Tinycc-devel] test 104 fails on windows: missing mkstemps
>  
> Well, the diffs are not really diffs. They just moved. Looks like `sort`
> didn't work as expected, or maybe it's some locale issue (mine is
> en_US.UTF-8 at cygwin, and en-US at windows).
>  
> A script should handle this too, possibly with LC_ALL=C (and make sure
> the reference file is generated with the same sort locale).
>  
> If someone could split it to program+script then I can test the test in
> various use cases and make adjustment if required (I'm terrible with
> Makefiles but reasonably useful with shell).
>  
> On Tuesday, June 18, 2019 9:50 PM, avih <[hidden email]> wrote:
>  
> After commit d39c49db  Remove empty conditional _WIN32 code
>
>
> and some hacking of the code (it's an unhealthy mix of basically running
> a shell script from a program compiled using tcc for windows), I get the
> following 2 diffs:
>
>
> +inst_extern_inline_postdeclared
> +inst_extern_inline_predeclared
>
>
>
> and
>
>
> -inst_extern_inline_postdeclared
> -inst_extern_inline_predeclared
>
>
>
> I'm running it in a cygwin environment and the tools (nm, sort, gawk)
> are cygwin tools, while the tested tcc is normal mingw tcc for windows
> (which I build in a reproducible way using my script).
>
>
>
> Regardless of these two diffs, I think the test should be composed of a
> program and a normal shell script (which uses mktemp, awk, sort etc), so
> that the paths are consistent between the tools.
>
>
> Also, the TCC path is hardcoded at the test, but in fact it's parametric
> at the makefile as $(TCC), so it's better to use that instead (but then
> there are forward/backward slash issues which need to be handled too,
> because system(...) in win32 expects backward slashes, but $(TCC) at the
> makefile has forward slashes). Making this a program + a script should
> implicitly solve this issue as well.
>
>
>
> After all, a working shell+tools is assumed for this test anyway, but
> the current way of using system(...) from a win32 program (compiled
> using tcc for windows) invokes a windows shell which can be inconsistent
> with the actual shell where `make` runs.
>
>
> Avi
>
>
>
>
>
>  
> On Tuesday, June 18, 2019 12:11 AM, avih <[hidden email]> wrote:
>  
> Hmm.. I now see that test 104 uses signal and nm, so it might require
> some effort to make it work on windows.
>  
> Nevertheless, considering the recent breakage specifically on windows
> from related code, I think it would be beneficial if this test could be
> made to work on windows,
>  
> On Monday, June 17, 2019 11:54 PM, avih <[hidden email]> wrote:
>  
> Wouldn't it be better to just create a known/fixed file instead?
> (assuming the test doesn't need explicitly mkstemps, I didn't look at
> its actual code).
>  
> On Monday, June 17, 2019 11:50 PM, Christian Jullien <[hidden email]>
> wrote:
>  
> Yes it has been previously reported.
>  
> Michael, as said in a private mail, I agree with you that this test
> should be skipped on Windows.
>  
> C.
>  
> *From:*Tinycc-devel
> [mailto:tinycc-devel-bounces+eligis=[hidden email]] *On Behalf Of
> *avih
> *Sent:* Monday, June 17, 2019 22:46
> *To:* Tinycc-devel Mailing List
> *Subject:* [Tinycc-devel] test 104 fails on windows: missing mkstemps
>  
> Test 104 log on windows (both tcc32 and tcc 64):
>  
> Test: 104_inline_test...
> --- 104_inline_test.expect      2019-06-17 23:42:00.162697100 +0300
> +++ 104_inline_test.output      2019-06-17 23:42:35.531550400 +0300
> @@ -1,25 +1,2 @@
> -extern_extern_postdeclared
> -extern_extern_postdeclared2
> -extern_extern_predeclared
> -extern_extern_predeclared2
> -extern_extern_prepostdeclared
> -extern_extern_prepostdeclared2
> -extern_extern_undeclared
> -extern_extern_undeclared2
> -extern_postdeclared
> -extern_postdeclared2
> -extern_predeclared
> -extern_predeclared2
> -extern_prepostdeclared
> -extern_undeclared
> -extern_undeclared2
> -inst2_extern_inline_postdeclared
> -inst2_extern_inline_predeclared
> -inst3_extern_inline_predeclared
> -inst_extern_inline_postdeclared
> -inst_extern_inline_predeclared
> -main
> -noinst_extern_inline_func
> -noinst_extern_inline_postdeclared
> -noinst_extern_inline_postdeclared2
> -noinst_extern_inline_undeclared
> +104_inline_test.c:30: warning: implicit declaration of function 'mkstemps'
> +tcc: error: undefined symbol 'mkstemps'
> make[1]: *** [Makefile:70: 104_inline_test.test] Error 1
> Test: 105_local_extern...
> make[1]: Target 'all' not remade because of errors.
>  
>  
>  
>  
>  
>
>

 

 

 

_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

grischka
[hidden email] wrote:
>   Pushed a slightly simpler version. Also, changed gawk to awk for wider
> compatibility.
>

Not bad, but I could not resist to spell out my own thoughts too ;)
https://repo.or.cz/tinycc.git/commitdiff/e6d8f9a8

-- gr


_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

avih
> [hidden email] wrote:
> > Pushed a slightly simpler version. Also, changed gawk to awk for wider
> > compatibility.
> >
>
> Not bad, but I could not resist to spell out my own thoughts too ;)
> https://repo.or.cz/tinycc.git/commitdiff/e6d8f9a8
>
> -- gr

Much better, thanks :)

Avi

_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Reply | Threaded
Open this post in threaded view
|

Re: test 104 fails on windows: missing mkstemps

avih





> > Not bad, but I could not resist to spell out my own thoughts too ;)
> > https://repo.or.cz/tinycc.git/commitdiff/e6d8f9a8
> >
> > -- gr
>
>
> Much better, thanks :)
>
> Avi

However, out-of-tree build/test now fails with "tcc: error: file '104+_inline.c' not found":

mkdir bld && cd bld && ../configure && make && make test

_______________________________________________
Tinycc-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel