[Scons-dev] Patch for review: add missing .itcl target to SWIG builder

[William Blevins]

>
> 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.Golding at 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 at scons.org
> https://pairlist2.pair.net/mailman/listinfo/scons-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://pairlist2.pair.net/pipermail/scons-dev/attachments/20141105/7d8cd3d5/attachment.html>