The test started out as a shell script. It could have been a pipeline,
succeeds, you get a success.
using tempfiles as glue. You can theoretically do the same from a shell
longer than the shell code.
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
> <
address@hidden> 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=
address@hidden] *On Behalf Of
> *avih
> *Sent:* Tuesday, June 18, 2019 21:01
> *To:*
address@hidden;
address@hidden; '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 <
address@hidden> 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 <
address@hidden> 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 <
address@hidden> 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 <
address@hidden>
> 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=
address@hidden] *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.
>
>
>
>
>
>
>