Re: [Scons-dev] Patch for review: add missing .itcl target to SWIG builder
Thanks William for the comments. The .itcl file is interpreted Tcl code, unlike Java/C/C++ it does not need to be built. Unfortunately I'm too busy to create the pull request right now :( I will add it to my backlog. The patch will also get some internal testing here (on scons 2.2.0), although I couldn't find another module in our source tree which uses the -itcl flag. Ben From: Scons-dev [mailto:scons-dev-boun...@scons.org] On Behalf Of William Blevins Sent: 06 November 2014 00:25 To: SCons developer list Subject: Re: [Scons-dev] Patch for review: add missing .itcl target to SWIG builder I have two concerns about this patch as it stands: 1) Compatibility impact on existing SConscript from changing the length of the returned target list. 2) The SWIG builder gets invoked through env.CFile() or env.CXXFile(), however only one of the returned targets is a C/C++ file. Unless you have a specific reason, I do not foresee either of these conditions causing issues. 1) Emitters return lists of elements by design, so this should be handled without concern. 2) Builders are chosen for Nodes per their scanner_key, so unless you explicitly add the .itcl extension to the CSuffixes list (or some other equivalent method), then the item will not be built with the C-builder action. If this file needs to be built in some way, then a build action needs to be created for it. The best way to get your patch looked at (sooner vs later) is to create a pull request @ https://bitbucket.org/scons/scons ; there should be some notes about the process @ http://www.scons.org/wiki/SconsMercurialWorkflows . V/R, William On Wed, Nov 5, 2014 at 4:35 PM, Ben Golding ben.gold...@synopsys.com wrote: After reading the responses to the other thread that I opened, I now understand that SideEffect() should not be used for files that I want to install, or indeed care about at all. I found a couple of cases where the SWIG builder misses some generated files from the returned targets, in generating Java and [incr Tcl] code. The Java case is a bit complex, since there can be many additional files; figuring out the names would amount to writing a C/C++ parser. I've opened a thread on the SWIG users list about this. When specifying the -itcl option, however, the output file seems to be always based on the input (foo.i = foo.itcl). I've attached a simple patch which changes the emitter to return foo.itcl in target[1]. There is no change to target[0] which is a C/C++ file. I have two concerns about this patch as it stands: 1) Compatibility impact on existing SConscript from changing the length of the returned target list. 2) The SWIG builder gets invoked through env.CFile() or env.CXXFile(), however only one of the returned targets is a C/C++ file. I suppose the above could be addressed by extra code to handle the .itcl extension in generate(), but I'm unsure exactly the right way to do this. I'm also unsure how the SConscript should look in that case, since the .itcl and .cpp files are generated at once. (The .itcl file is something like a side effect, but we do care about it.) So returning the additional list item from CXXFile() seems a pragmatic solution. Comments/improvements are welcome. Regards Ben ___ Scons-dev mailing list Scons-dev@scons.org https://pairlist2.pair.net/mailman/listinfo/scons-dev ___ Scons-dev mailing list Scons-dev@scons.org https://pairlist2.pair.net/mailman/listinfo/scons-dev
Re: [Scons-dev] Likely bug - installing side effect files
It seems one possible fix for this bug could be a warning/error if the user tries to Install() a side-effect file, and expanding/clarifying the docs for SideEffect(). Also, if Install() is permitted with a warning, the current behaviour (silently ignoring the file) must be fixed. A few other comments/questions come to mind: 1) If the contents of a side-effect file are indeterminate, how does telling SCons about this file help to have a deterministic (or convergent) build? What purpose does SideEffect() have at all? What would be the remaining valid use-cases, if Install() is not permitted? 2) The example given in docs of .pdb file is an interesting one. It looks like SideEffect() is not actually used in msvc.py. In fact, this indeterminate behaviour depends on whether the option -Fd [1] is used, what filename is specified with -Fd, and the debug information format option [2]. It is both possible and desirable, particularly in a parallel build, to either use -Z7 (foo.cc = foo.obj; no .pdb), or to use -Zi / -ZI with -Fd and a suitably unique filename (foo.cc = foo.obj, foo.pdb). However, using -Zi without explicit -Fd gives something like (foo.cc = foo.obj, vc100.pdb). (There are probably multiple bugs in the MSVC support described above...) For me, the interesting question is 1), and whether a one size fits all side-effect concept can ever be really useful in complex cases such as 2). It seems clear that for a convergent build, the builder needs to fully understand the complexities of such issues and do the right thing in all cases. The right thing may be to mark the file vc100.pdb as unstable, or just ignore it - but SideEffect() doesn't help there. While foo.pdb should be a target along with foo.obj, so it can be installed. Regards Ben [1] http://msdn.microsoft.com/en-us/library/9wst99a9.aspx [2] http://msdn.microsoft.com/en-us/library/958x11bc.aspx ___ Scons-dev mailing list Scons-dev@scons.org https://pairlist2.pair.net/mailman/listinfo/scons-dev
[Scons-dev] Likely bug - installing side effect files
Minimal reproducer:
tgt = Command('tgt', 'src', 'touch $TARGET sf0 sf1')
sf = SideEffect([ 'sf0', 'sf1' ], tgt)
Install('dir', tgt + sf)
Running the above with 'scons -j1 -Q' works correctly.
When using -j2 (or greater), the file 'sf1' is created, but not copied to
'dir/sf1'.
Any suggestions for a fix or workaround would be really useful.
I tried adding
Depends('dir', sf[1])
but this only causes the issue to move to 'dir/sf0' instead.
RHEL 5.7 x64 / python 2.7.2 / scons 2.3.4
Regards
Ben Golding
___
Scons-dev mailing list
Scons-dev@scons.org
https://pairlist2.pair.net/mailman/listinfo/scons-dev
Re: [Scons-dev] Likely bug - installing side effect files
Thanks Gary.
http://scons.tigris.org/issues/show_bug.cgi?id=2982
I started to add some additional trace to Taskmaster.py, and compare the logs
from sequential vs. parallel builds.
The bug seems to relate to how the node states are used for the side-effect
nodes:
- In executed_with_callbacks(), the side-effect state is set to NODE_NO_STATE.
- In postprocess(), the logic to decrement the parent ref count is only active
for a side-effect node with state NODE_EXECUTING.
Although this helped me to understand the bug a bit better, I'm not sure what
is the correct fix.
Regards
Ben
-Original Message-
From: Scons-dev [mailto:scons-dev-boun...@scons.org] On Behalf Of Gary
Oberbrunner
Sent: 30 October 2014 14:30
To: SCons developer list
Subject: Re: [Scons-dev] Likely bug - installing side effect files
On Thu, Oct 30, 2014 at 9:34 AM, Ben Golding ben.gold...@synopsys.com wrote:
tgt = Command('tgt', 'src', 'touch $TARGET sf0 sf1')
sf = SideEffect([ 'sf0', 'sf1' ], tgt)
Install('dir', tgt + sf)
I can reproduce this bug. Please file a ticket! The dependency tree
looks OK, and I can't easily see the cause of the bug.
Adding sf0 and sf1 to the targets list instead of as side effects makes it work.
--
Gary
___
Scons-dev mailing list
Scons-dev@scons.org
https://pairlist2.pair.net/mailman/listinfo/scons-dev
___
Scons-dev mailing list
Scons-dev@scons.org
https://pairlist2.pair.net/mailman/listinfo/scons-dev
