Bill/Jason, I took a look into adding default behavior for nodes without a scanner match. It's my interpretation of our conversation yesterday. My meeting ran WAY over its time box, so I kinda disappeared. Sorry.
Please see here: https://bitbucket.org/scons/scons/pull-requests/237/issue-2264-cross-language-scanner-support/diff#comment-8606867 ; I think this addresses the concerns. Also, I might have found an issue that already existed. Please see: https://bitbucket.org/scons/scons/pull-requests/237/issue-2264-cross-language-scanner-support/diff#comment-8631194 ; maybe I am paranoid. Also, I added comments to all the test changes in the pull request to try and explain why they were required (to help things move along). V/R, William On Wed, Jul 29, 2015 at 10:50 PM, William Blevins <[email protected]> wrote: > > > On Wed, Jul 29, 2015 at 10:05 PM, Jason Kenny <[email protected]> wrote: > >> Ya I see this... >> >> So I trying to take a little time tonight and figure out what happens.. >> Could use some help. >> >> I currently looking at test/CPPSUFFIXES.py >> >> This fails at line 107. From what I can tell the test expects output of >> >> C:\Python27\python.exe mycc.py test1.o test1.c >> >> but instead we get >> >> C:\Python27\python.exe mycc.py test1.o test1.c >> Install file: "test1.c" as "test1_c" >> Install file: "test1.h" as "test1_h" >> >> This happens because foo.h was changed causing these files to be copied >> again. >> >> So problems I have with this test. >> >> 1) no one would copy headers that depend on a header not passed to the >> install area. I am not sure if we should treat <> different from “” >> includes. It a common practice to use “” for headers in your library and <> >> for stuff outside your library. I don’t this really helps either way >> 2) the copy/install builder does not generate new file if the source is >> different. What we have here is a good scanner that would do the right >> thing if the “binary” needs to be rebuilt, but redundant is the copy has to >> happen. >> > > Gary and I were of the same opinion. The functionality is correct and > matches all other scanner/builder behavior, but it is inefficient... > > >> The issue in a way is that SCons makes the installed test1_c out of date >> because foo.h is out of date, when in fact this is a case in which we would >> want to update logic to care more about the csig of the source being the >> same as what is stored and not doing anything as technically this is >> correct behavior for copy like builders and ignoring that the scanner >> header is out of date. >> >> > > >> I see few solutions to this: >> >> 1) we say current behavior is correct and update the test. >> > > This is the easiest, but I was hoping to resolve this performance issue. > > >> 2) we make a change to allow a install/copy builder to say the scanner >> items are Required() which remove the Depends() state >> > > I looked into doing this, but I didn't see an easy way to do this. I made > some notes: > https://bitbucket.org/scons/scons/pull-requests/237/issue-2264-cross-language-scanner-support/diff#comment-8502930 > > >> 3) we change builder to apply a custom sig check for the install nodes to >> ignore that a file dependent from the scanner. >> > > This seems like a reasonable approach unless this means updating every > scanner. The fundamental problem here is that language scanners already > exist that exhibit correct behavior. I don't want to duplicate any part of > the scanner code. I feel like going this route will require some level of > duplication. > > >> 4) remove the scanner from the install builder as we only want to copy >> files that change >> > > I assume this is functional equivalent to disabling recursive scanning in > the install builder. I already considered this and think its the best > choice. See some notes here about our previous conversation: > https://bitbucket.org/scons/scons/pull-requests/237/issue-2264-cross-language-scanner-support/diff#comment-8521558 > > With that change only 3 tests are left failing for me. See copa-pasta > snippet below: > > test/IDL/IDLSUFFIXES.py # If I understand the test correctly, this test > checks to see that the bug we are trying to fix works??? > test/explain/basic.py # Contains code with build dependencies on installed > files as sources: is this something SCons explicitly supports? > test/explain/save-info.py # Contains code with build dependencies on > installed files as sources: is this something SCons explicitly supports? > > Please pay special attention to test/IDL/IDLSIFFIXES.py because this test > requires the recursive scanner behavior to work which leads me to believe > that the recursive scanner problem already existed, and there used to be a > hack for this with the CScanner... > > >> >> I lean toward 4) logic given I how I understand the CCopy builder in >> Parts works. and that the install builder for the most part is a copy >> builder that adds nodes to a list to be used by the Scons version of the >> packaging builders. >> > > Agreed. See above. > > >> >> Jason >> >> >> *From:* William Blevins <[email protected]> >> *Sent:* Wednesday, July 29, 2015 7:19 PM >> *To:* SCons developer list <[email protected]> >> *Subject:* Re: [Scons-dev] Cross-language support >> >> Jason. >> >> FYI. The failing tests you listed match the list from June. The >> action-test.py may be new. >> >> >> https://bitbucket.org/scons/scons/pull-requests/237/issue-2264-cross-language-scanner-support/diff#comment-7323143 >> >> V/R, >> William >> >> On Wed, Jul 29, 2015 at 8:16 PM, William Blevins <[email protected]> >> wrote: >> >>> Jason, >>> >>> As previously stated in the pull request comments, I expect some tests >>> to fail currently because of the recursive header install issue (topic #1): >>> <lang>SUFFIXES, HeaderInstall.py. The other tests may or may not be >>> related. >>> >>> There is more value in looking at those tests first. It may also be >>> valuable to go back 3-4 commits to the closest ancestor of the default >>> branch and run tests there, so that we can see if some of the tests failed >>> previously. Also, if we think its a problem of the current location of >>> that divergent head, then we can see if a rebase fixes some of them. >>> >>> V/R, >>> William >>> >>> On Wed, Jul 29, 2015 at 7:09 PM, Jason Kenny <[email protected]> wrote: >>> >>>> ok not any better... >>>> >>>> >>>> Failed the following 16 tests: >>>> test\Batch\action-changed.py >>>> test\CPPSUFFIXES.py >>>> test\DSUFFIXES.py >>>> test\Fortran\FORTRANSUFFIXES.py >>>> test\HeaderInstall.py >>>> test\MSVS\vs-9.0-exec.py >>>> test\Win32\mingw.py >>>> test\exitfns.py >>>> test\implicit\IMPLICIT_COMMAND_DEPENDENCIES.py >>>> test\import.py >>>> test\long-lines\signature.py >>>> test\scons-time\run\config\python.py >>>> test\scons-time\run\option\python.py >>>> test\sconsign\script\SConsignFile.py >>>> test\sconsign\script\Signatures.py >>>> test\sconsign\script\no-SConsignFile.py >>>> >>>> I would need to dig down to see what is failing and why >>>> Jason >>>> >>>> >>>> *From:* Jason Kenny <[email protected]> >>>> *Sent:* Wednesday, July 29, 2015 5:38 PM >>>> *To:* SCons developer list <[email protected]> >>>> *Subject:* Re: [Scons-dev] Cross-language support >>>> >>>> ok I have it fixed... I am rerunning the tests now >>>> Jason >>>> >>>> *From:* Jason Kenny <[email protected]> >>>> *Sent:* Wednesday, July 29, 2015 5:30 PM >>>> *To:* SCons developer list <[email protected]> >>>> *Subject:* Re: [Scons-dev] Cross-language support >>>> >>>> I must have messed up. I only see debugCount and JniHeaderDir >>>> bookmarks at the moment. >>>> >>>> Jason >>>> >>>> *From:* William Blevins <[email protected]> >>>> *Sent:* Wednesday, July 29, 2015 4:50 PM >>>> *To:* SCons developer list <[email protected]> >>>> *Subject:* Re: [Scons-dev] Cross-language support >>>> >>>> That's a SCons fork. Not an HG branch (in case its a terminology >>>> problem). Make sure you are using the CrossLanguage bookmark. >>>> >>>> V/R, >>>> William >>>> >>>> On Wed, Jul 29, 2015 at 5:45 PM, Jason Kenny <[email protected]> >>>> wrote: >>>> >>>>> branch *SCons_20150323 * >>>>> >>>>> I can will setup from scratch again and try again. >>>>> >>>>> Jason >>>>> >>>>> *From:* William Blevins <[email protected]> >>>>> *Sent:* Wednesday, July 29, 2015 4:38 PM >>>>> *To:* SCons developer list <[email protected]> >>>>> *Subject:* Re: [Scons-dev] Cross-language support >>>>> >>>>> Jason, >>>>> >>>>> Are you sure you are using the correct commit revision? >>>>> >>>>> None of those tests are failing on my Linux distro, and I haven't >>>>> modified any of those tests either. Plus, tests that I am expecting to >>>>> fail are apparently passing. >>>>> >>>>> V/R, >>>>> William >>>>> >>>>> On Wed, Jul 29, 2015 at 5:06 PM, Jason Kenny <[email protected]> >>>>> wrote: >>>>> >>>>>> So a quick pass of the tests has these failures >>>>>> >>>>>> Failed the following 15 tests: >>>>>> test\Batch\action-changed.py >>>>>> test\MSVS\vs-9.0-exec.py >>>>>> test\Win32\mingw.py >>>>>> test\exitfns.py >>>>>> test\implicit\IMPLICIT_COMMAND_DEPENDENCIES.py >>>>>> test\import.py >>>>>> test\long-lines\signature.py >>>>>> test\option\debug-count.py >>>>>> test\option\debug-multiple.py >>>>>> test\option\debug-objects.py >>>>>> test\scons-time\run\config\python.py >>>>>> test\scons-time\run\option\python.py >>>>>> test\sconsign\script\SConsignFile.py >>>>>> test\sconsign\script\Signatures.py >>>>>> test\sconsign\script\no-SConsignFile.py >>>>>> >>>>>> *From:* Jason Kenny <[email protected]> >>>>>> *Sent:* Wednesday, July 29, 2015 2:32 PM >>>>>> *To:* SCons developer list <[email protected]> >>>>>> *Subject:* Re: [Scons-dev] Cross-language support >>>>>> >>>>>> Ok let me update the branch and re-run everything. >>>>>> >>>>>> Jason >>>>>> >>>>>> *From:* William Blevins <[email protected]> >>>>>> *Sent:* Wednesday, July 29, 2015 10:30 AM >>>>>> *To:* SCons developer list <[email protected]> >>>>>> *Subject:* Re: [Scons-dev] Cross-language support >>>>>> >>>>>> Jason, >>>>>> >>>>>> I'm not sure. I remember that you were helping me look into the >>>>>> recursive Install behavior. Plus, possible parts incompatibility? >>>>>> >>>>>> I haven't ran the tests on a windows box at all. I don't expect >>>>>> anything new, but it should be done before it ships :) >>>>>> >>>>>> V/R, >>>>>> William >>>>>> >>>>>> On Wed, Jul 29, 2015 at 8:44 AM, Jason Kenny <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> Which MS toolchain on windows do we need tested? I can do a test >>>>>>> run this afternoon. >>>>>>> >>>>>>> Jason >>>>>>> >>>>>>> *From:* William Blevins <[email protected]> >>>>>>> *Sent:* Wednesday, July 29, 2015 1:11 AM >>>>>>> *To:* SCons developer list <[email protected]> >>>>>>> *Subject:* Re: [Scons-dev] Cross-language support >>>>>>> >>>>>>> Happy with it* >>>>>>> >>>>>>> Until the review is complete, and I can perhaps get a few more >>>>>>> guinea pigs to try it out, it's hard to make a concrete projection. I >>>>>>> have >>>>>>> 3 items listed out that need to be given a thumbs up/down at a minimum: >>>>>>> >>>>>>> * Item #1 is still mostly outstanding. I'm not sure how to address, >>>>>>> please see the discussion Jason and I started a few weeks ago under the >>>>>>> pull request comments. >>>>>>> >>>>>>> * Item #2 requires no changes (to my knowledge), but someone with >>>>>>> more QT knowledge may say otherwise. >>>>>>> >>>>>>> * I may already have most of the patch for Item #3 (IE. no scanner >>>>>>> for key) based on today's feedback. >>>>>>> >>>>>>> I also still need to request test runs on Windows, and for >>>>>>> toolchains that I may not have installed or instructions to install them >>>>>>> (EG. D). >>>>>>> >>>>>>> V/R, >>>>>>> William >>>>>>> >>>>>>> On Wed, Jul 29, 2015 at 1:58 AM, William Blevins < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> As soon as we are happen with it :) >>>>>>>> >>>>>>>> On Wed, Jul 29, 2015 at 12:30 AM, Bill Deegan < >>>>>>>> [email protected]> wrote: >>>>>>>> >>>>>>>>> O.k. let me push 2.3.5 with the visual studio 2015 stuff. >>>>>>>>> Then we'll changed to 2.4 merge slots. stabilize. release. >>>>>>>>> Then this code? (2.5?) >>>>>>>>> >>>>>>>>> -Bill >>>>>>>>> >>>>>>>>> On Tue, Jul 28, 2015 at 6:33 PM, Jason Kenny <[email protected]> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> I have been using the slots drop for a while with Parts. I >>>>>>>>>> think it is ready. It does have a notable improvement in speed and >>>>>>>>>> memory >>>>>>>>>> size. I would before getting this out as officially earlier than >>>>>>>>>> later. >>>>>>>>>> >>>>>>>>>> Jason >>>>>>>>>> >>>>>>>>>> *From:* William Blevins <[email protected]> >>>>>>>>>> *Sent:* Tuesday, July 28, 2015 8:31 PM >>>>>>>>>> *To:* Dirk Bächle <[email protected]> ; SCons developer list >>>>>>>>>> <[email protected]> >>>>>>>>>> *Subject:* Re: [Scons-dev] Cross-language support >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Tue, Jul 28, 2015 at 7:16 PM, Dirk Bächle <[email protected]> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> Hi guys, >>>>>>>>>>> >>>>>>>>>>> sorry for chiming in so late. >>>>>>>>>>> >>>>>>>>>>> On 28.07.2015 23:44, Gary Oberbrunner wrote: >>>>>>>>>>> >>>>>>>>>>>> Yes, that's how we've done it in the past. Sounds like doing >>>>>>>>>>>> it at the same time as slots would be perfect. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> Doing this in parallel with the "slots" change sounds good to me >>>>>>>>>>> too. +1 >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> I'm not opposed to releasing in the same update as slots, but >>>>>>>>>> since the cross language code reviews haven't been finished, I don't >>>>>>>>>> want >>>>>>>>>> to delay slots since it is ready now. >>>>>>>>>> >>>>>>>>>> The pessimist in me as sees doing two major enhancements in the >>>>>>>>>> same release as higher risk; I don't foresee any issues, but its >>>>>>>>>> worth the >>>>>>>>>> thought if the release overhead isn't too bad. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> -- Gary >>>>>>>>>>>> >>>>>>>>>>>> On Tue, Jul 28, 2015 at 2:01 PM, Bill Deegan < >>>>>>>>>>>> [email protected] <mailto:[email protected]>> >>>>>>>>>>>> wrote: >>>>>>>>>>>> >>>>>>>>>>>> Gary, >>>>>>>>>>>> >>>>>>>>>>>> For such a change we should bump the second digit? >>>>>>>>>>>> 2.4? >>>>>>>>>>>> >>>>>>>>>>>> I agree we should not turn down a change because it will >>>>>>>>>>>> cause rebuilds where the past didn't as long as it is now more >>>>>>>>>>>> correct >>>>>>>>>>>> (which it should be with this change). >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Yes, "forward" is the way to go. ;) >>>>>>>>>>> >>>>>>>>>>> Also agree we should be verbose in our notification of the >>>>>>>>>>>> impacts of the new change to avoid (as much as we can) "surprises". >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> I think (better: hope) we did a good enough job for the "slots" >>>>>>>>>>> stuff on this. For the scanner changes, I see them more like a >>>>>>>>>>> fix...so a >>>>>>>>>>> single announcement should be sufficient? >>>>>>>>>>> >>>>>>>>>>> Finally, and just in case I haven't done so already, I'd like to >>>>>>>>>>> thank William for all the work he's done on this issue. I couldn't >>>>>>>>>>> help as >>>>>>>>>>> much as I would've liked, but with Gary's support you tackled this >>>>>>>>>>> down and >>>>>>>>>>> brought it to a good end. Kudos to you...bravo! >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> Truly appreciated. I have spent a lot of time on this issue >>>>>>>>>> despite what one might expect from the number of lines of code. >>>>>>>>>> >>>>>>>>>> It was honestly my first time working in a "real" python >>>>>>>>>> environment outside of scripting, and the SCons code base is rather >>>>>>>>>> complex. It instills me greater appreciation for the work that has >>>>>>>>>> already >>>>>>>>>> done. >>>>>>>>>> >>>>>>>>>> I also want to thank everyone here for their help past and >>>>>>>>>> future, but don't pat me on the back until its done. I might get >>>>>>>>>> lazy :) >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Best regards, >>>>>>>>>>> >>>>>>>>>>> Dirk >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> _______________________________________________ >>>>>>>>>>> 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 >>>>>>>>>> >>>>>>>>>> _______________________________________________ >>>>>>>>>> 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 >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>>> ------------------------------ >>>>>>> _______________________________________________ >>>>>>> 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 >>>>>>> >>>>>>> >>>>>> ------------------------------ >>>>>> _______________________________________________ >>>>>> 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 >>>>>> >>>>>> _______________________________________________ >>>>>> 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 >>>>> >>>>> _______________________________________________ >>>>> 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 >>>> ------------------------------ >>>> _______________________________________________ >>>> 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 >>>> >>>> _______________________________________________ >>>> 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 >> >> >> _______________________________________________ >> 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
