Re: Bug in MSVC_BATCH handling

Eric Fahlgren <[email protected]> Sun, 15 Dec 2019 13:24:10 -0800
Newsgroups gmane.comp.programming.tools.scons.devel
Message-ID <CAP2Qz+W4CgeBrsCt1O83_Yng19uQ7qzRpRucBSzfTcoCYTDDmA@mail.gmail.com>
--===============6981173743459130510==
Content-Type: multipart/alternative; boundary="000000000000f0f9aa0599c4b71f"

--000000000000f0f9aa0599c4b71f
Content-Type: text/plain; charset="UTF-8"

I'm not a git user, is there a tutorial on how to create a PR?

On Sun, Dec 15, 2019 at 12:49 PM Bill Deegan <bill-cJFiu+DHMVC5azolltMz9laTQe2KTcn/@public.gmane.org>
wrote:

> Generally issues like this should go to the users mailing list.
> (Mainly so searching users mailing list will find issues run into by
> users).
>
> Good catch.
> Any chance you can put together a PR with a test?
> -Bill
>
> On Sun, Dec 15, 2019 at 12:13 PM Eric Fahlgren <[email protected]>
> wrote:
>
>> Py 3.7 64, Win 10 64
>> SCons 3.1.1 (but same as master in this case)
>>
>> While cleaning up some old scripts, I uncovered a bug that I've been
>> working around for who knows how long and finally isolated it.
>>
>> Problem: When MSVC_BATCH is enabled globally, files with different target
>> name are built individually, but ignoring the specified $TARGET name.
>> Here's a little example, note the first argument to the "cl" output.
>>
>> Input:
>> env = Environment(MSVC_BATCH=True)
>> bug32 = env.Object(target='file32', source='file.cpp')
>>
>> Output:
>> cl  /FoWIN32\src\\  /c  WIN32\src\file.cpp
>>
>> If you explicitly suppress batching in the Object, then it works fine.
>> Input:
>> obj32 = env.Object(target='file32', source='file.cpp', MSVC_BATCH=False)
>> Output:
>> cl  /FoWIN32\src\file32.obj  /c  WIN32\src\file.cpp
>>
>> I chased this to the logic in Tool.msvc, the function msvc_output_flag
>> where it's checking the state of the MSVC_BATCH variable, and ignores the
>> difference in base name that the function just above (msvc_batch_key)
>> performs, see lines 168-173.  My hack, which seems to work, is to change
>> line 191 in msvc.py from:
>>
>> 191:     if 'MSVC_BATCH' not in env or env.subst('$MSVC_BATCH') in ('0',
>> 'False', '', None):
>>
>> to use the already-working logic in the batch key function like this:
>>
>> 191:    if msvc_batch_key(None, env, target, source) is None:
>>
>> I tried it on single file and multi-file compilations, and from my
>> cursory playing around, this keeps the batched files together and separates
>> out those with different target base names, but then I didn't try all cases
>> or check any other downstream code in the pipeline to see if this would
>> break anything there...
>>
>> _______________________________________________
>> Scons-dev mailing list
>> [email protected]
>> https://pairlist2.pair.net/mailman/listinfo/scons-dev
>>
> _______________________________________________
> Scons-dev mailing list
> [email protected]
> https://pairlist2.pair.net/mailman/listinfo/scons-dev
>

--000000000000f0f9aa0599c4b71f
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div class=3D"gmail_default" style=3D"font-family:arial,sa=
ns-serif;color:#000000">I&#39;m not a git user, is there a tutorial on how =
to create a PR?<br></div></div><br><div class=3D"gmail_quote"><div dir=3D"l=
tr" class=3D"gmail_attr">On Sun, Dec 15, 2019 at 12:49 PM Bill Deegan &lt;<=
a href=3D"mailto:bill-cJFiu+DHMVC5azolltMz9laTQe2KTcn/@public.gmane.org">bill-cJFiu+DHMVC5azolltMz9laTQe2KTcn/@public.gmane.org</a>&g=
t; wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0p=
x 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div d=
ir=3D"ltr"><div>Generally issues like this should go to the users mailing l=
ist.</div><div>(Mainly so searching users mailing list will find issues run=
 into by users).</div><div><br></div><div>Good catch.</div><div>Any chance =
you can put together a PR with a test?</div><div>-Bill<br></div></div><br><=
div class=3D"gmail_quote"><div dir=3D"ltr" class=3D"gmail_attr">On Sun, Dec=
 15, 2019 at 12:13 PM Eric Fahlgren &lt;<a href=3D"mailto:ericfahlgren@gmai=
l.com" target=3D"_blank">[email protected]</a>&gt; wrote:<br></div><bl=
ockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-lef=
t:1px solid rgb(204,204,204);padding-left:1ex"><div dir=3D"ltr"><div class=
=3D"gmail_default" style=3D"font-family:arial,sans-serif;color:rgb(0,0,0)">=
Py 3.7 64, Win 10 64<br></div><div class=3D"gmail_default" style=3D"font-fa=
mily:arial,sans-serif;color:rgb(0,0,0)">SCons 3.1.1 (but same as master in =
this case)<span><span datetime=3D"2019-04-27T19:39:27Z" title=3D"Apr 27, 20=
19, 12:39 PM PDT"></span>
        </span>

</div><div class=3D"gmail_default" style=3D"font-family:arial,sans-serif;co=
lor:rgb(0,0,0)"><br></div><div class=3D"gmail_default" style=3D"font-family=
:arial,sans-serif;color:rgb(0,0,0)">While cleaning up some old scripts, I u=
ncovered a bug that I&#39;ve been working around for who knows how long and=
 finally isolated it.</div><div class=3D"gmail_default" style=3D"font-famil=
y:arial,sans-serif;color:rgb(0,0,0)"><br></div><div class=3D"gmail_default"=
 style=3D"font-family:arial,sans-serif;color:rgb(0,0,0)">Problem: When MSVC=
_BATCH is enabled globally, files with different target name are built indi=
vidually, but ignoring the specified $TARGET name.=C2=A0 Here&#39;s a littl=
e example, note the first argument to the &quot;cl&quot; output.<br></div><=
div class=3D"gmail_default" style=3D"font-family:arial,sans-serif;color:rgb=
(0,0,0)"><br></div><div class=3D"gmail_default" style=3D"font-family:arial,=
sans-serif;color:rgb(0,0,0)">Input:</div><div class=3D"gmail_default" style=
=3D"font-family:arial,sans-serif;color:rgb(0,0,0)">env =3D Environment(MSVC=
_BATCH=3DTrue)<br></div>
<div class=3D"gmail_default" style=3D"font-family:arial,sans-serif;color:rg=
b(0,0,0)">
bug32 =3D env.Object(target=3D&#39;file32&#39;, source=3D&#39;file.cpp&#39;=
)<br></div><div class=3D"gmail_default" style=3D"font-family:arial,sans-ser=
if;color:rgb(0,0,0)">
</div><div><span class=3D"gmail_default" style=3D"font-family:arial,sans-se=
rif;color:rgb(0,0,0)"><br></span></div><div><span class=3D"gmail_default" s=
tyle=3D"font-family:arial,sans-serif;color:rgb(0,0,0)">Output:<br></span></=
div><div><span class=3D"gmail_default" style=3D"font-family:arial,sans-seri=
f;color:rgb(0,0,0)">cl=C2=A0 /FoWIN32\src\\=C2=A0 /c=C2=A0 WIN32\src\file.c=
pp</span></div><div><br></div><div><div style=3D"font-family:arial,sans-ser=
if;color:rgb(0,0,0)" class=3D"gmail_default">If you explicitly suppress bat=
ching in the Object, then it works fine.
<div class=3D"gmail_default" style=3D"font-family:arial,sans-serif;color:rg=
b(0,0,0)">Input:<br></div><div class=3D"gmail_default" style=3D"font-family=
:arial,sans-serif;color:rgb(0,0,0)">obj32 =3D env.Object(target=3D&#39;file=
32&#39;, source=3D&#39;file.cpp&#39;, MSVC_BATCH=3DFalse)<br></div><div cla=
ss=3D"gmail_default" style=3D"font-family:arial,sans-serif;color:rgb(0,0,0)=
">
</div><div><span class=3D"gmail_default" style=3D"font-family:arial,sans-se=
rif;color:rgb(0,0,0)"></span>Output:<br></div><div>
<div><span class=3D"gmail_default" style=3D"font-family:arial,sans-serif;co=
lor:rgb(0,0,0)">cl=C2=A0 /FoWIN32\src\file32.obj=C2=A0 /c=C2=A0 WIN32\src\f=
ile.cpp</span></div>

</div></div></div><div><br></div><div><div style=3D"font-family:arial,sans-=
serif;color:rgb(0,0,0)" class=3D"gmail_default">I chased this to the logic =
in Tool.msvc, the function msvc_output_flag where it&#39;s checking the sta=
te of the MSVC_BATCH variable, and ignores the difference in base name that=
 the function just above (msvc_batch_key) performs, see lines 168-173.=C2=
=A0 My hack, which seems to work, is to change line 191 in msvc.py from:</d=
iv><div style=3D"font-family:arial,sans-serif;color:rgb(0,0,0)" class=3D"gm=
ail_default"><br></div><div style=3D"font-family:arial,sans-serif;color:rgb=
(0,0,0)" class=3D"gmail_default">191: =C2=A0=C2=A0=C2=A0
    <span>if</span> <span><span>&#39;</span>MSVC_BATCH<span>&#39;</span></s=
pan> <span>not</span> <span>in</span> env <span>or</span> env.<span>subst</=
span>(<span><span>&#39;</span>$MSVC_BATCH<span>&#39;</span></span>) <span>i=
n</span> (<span><span>&#39;</span>0<span>&#39;</span></span>, <span><span>&=
#39;</span>False<span>&#39;</span></span>, <span><span>&#39;</span><span>&#=
39;</span></span>, <span>None</span>):

</div></div><div><br></div><div><div style=3D"font-family:arial,sans-serif;=
color:rgb(0,0,0)" class=3D"gmail_default">to use the already-working logic =
in the batch key function like this:<br></div><div style=3D"font-family:ari=
al,sans-serif;color:rgb(0,0,0)" class=3D"gmail_default"><br></div><div styl=
e=3D"font-family:arial,sans-serif;color:rgb(0,0,0)" class=3D"gmail_default"=
>191:=C2=A0 =C2=A0 if msvc_batch_key(None, env, target, source) is None:</d=
iv></div><div><br></div><div><div style=3D"font-family:arial,sans-serif;col=
or:rgb(0,0,0)" class=3D"gmail_default">I tried it on single file and multi-=
file compilations, and from my cursory playing around, this keeps the batch=
ed files together and separates out those with different target base names,=
 but then I didn&#39;t try all cases or check any other downstream code in =
the pipeline to see if this would break anything there...</div><br></div></=
div>
_______________________________________________<br>
Scons-dev mailing list<br>
<a href=3D"mailto:[email protected]" target=3D"_blank">[email protected]=
g</a><br>
<a href=3D"https://pairlist2.pair.net/mailman/listinfo/scons-dev" rel=3D"no=
referrer" target=3D"_blank">https://pairlist2.pair.net/mailman/listinfo/sco=
ns-dev</a><br>
</blockquote></div>
_______________________________________________<br>
Scons-dev mailing list<br>
<a href=3D"mailto:[email protected]" target=3D"_blank">[email protected]=
g</a><br>
<a href=3D"https://pairlist2.pair.net/mailman/listinfo/scons-dev" rel=3D"no=
referrer" target=3D"_blank">https://pairlist2.pair.net/mailman/listinfo/sco=
ns-dev</a><br>
</blockquote></div>

--000000000000f0f9aa0599c4b71f--

--===============6981173743459130510==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Scons-dev mailing list
[email protected]
https://pairlist2.pair.net/mailman/listinfo/scons-dev

--===============6981173743459130510==--