[Scons-dev] SCons cross-language support

Thu Aug 27 12:31:54 EDT 2015

All,

I had to do another pull request because bitbucket couldn't figure out how
to correctly update my previous pull request.

Please see
https://bitbucket.org/scons/scons/pull-requests/244/issue-2264-cross-language-scanner-support/diff

The latest request includes the update the Install performance issue.  I
have some questions as comments on the file diff, but this should be ready
for an initial announcement, so that interested parties can take a peek.

V/R,
William

On Wed, Jun 3, 2015 at 7:31 PM, William Blevins <wblevins001 at gmail.com>
wrote:

> It seems that the current consensus is to update the tests to catch the
> additional stdout for Case A or update how the stdout is parsed so
> that changes to this behavior only hit the Install test and not other tests
> that happen to call the Install command.  This will affect about 10 tests
> (1-2 lines each) since all the builder tests are more or less copy-pastes
> of each other....
> On Wed, Jun 3, 2015 at 6:09 PM, Kenny, Jason L <jason.l.kenny at intel.com>
> wrote:
>
>> << If I understand your vote correctly, you want Install targets
>> non-recursive; thus, fixing Case A and removing test coverage of Case B.
>>
>>
>>
>> I would vote that all scanner should be recursive.
>>
>>
>>
>> From what I read on the B case is that it fails because it wants to
>> non-recusive scanner. ( maybe I read this wrong.. but the test passes with
>> your fix when before it failed… ie the test was expected to fail to succeed)
>>
>
> It works with and without my patch. Case B tests do not pass with
> non-recursive Install builder scanning.  Since we are in agreement to let
> Case A behave as is then Case B tests will continue to be happy campers.
>
>
>> Both case A and case B work (ie the way I think they should) because of
>> recursive scans. The issue of A being copied if B changes I understand as
>> being “wasteful” because it was copied ( using hard links helps with
>> this..ie why I have ccopy() in parts) but if this case was a.h to a.h.l was
>> a tool running to make a.h.l then it would not be as B.h could have
>> affected the output of a.h.l. It was just convenient that coping does not
>> have this issue, I think making the general scanner in some way so Scons
>> would not copy A.h would by over optimizing (ie to special case).
>>
>>
>>
>> I don’t see any case in which install should forget anything. The two
>> reasons for install are to define node to be packages and to make an
>> install sandbox for safe development work. Any changes in the build we
>> would want reflexed in the install sandbox. Install does not use the
>> Require() logic which is to say it expect it to exist but don’t rebuild me
>> if the source changes.
>>
>>
>>
>> As a side not one item that I have with SCons is the copy actions. I have
>> in general had to move to have a common copy builder ( ie the CCopy in
>> Parts) to allow consistent and constant behavior when coping items ( ie
>> directory or files). I even override the install builder in SCons to help
>> this as the last round of issue with “installing” a directory and Scons
>> calling the hidden mkdir actions for a node and saying it was done ( when
>> none of the contents had been built or copied) I only bring this up as the
>> issue in my mind with scanner are twofold:
>>
>>
>>
>> 1)      Some issues with recursion ( which you are addressing)
>>
>> 2)      Some issue with complex subst() with path variable not being
>> handed correctly ( I fixed this I Parts)
>>
>>
>>
>> I don’t believe we want by default scons to make possible incorrect
>> rebuilds or failed fist pass builds because of on purpose reduce dependency
>> scanners. The issue with copy vs install I did not ever understand as copy
>> and forget. That seems to suggest to me that you would have a built that
>> might be bad as some bit did not get updated. Who would want that as a
>> develop? I have to deal with that hair pulling time wasting annoyance when
>> forced to use autoconf/make already.
>>
>>
>>
>> I do believe you are making a case indirectly that we should have a
>> builder and that can take a scanner that would say the this dependent files
>> that are returned are Require() so they exist, but don’t force rebuild vs
>> the current logic of rebuild the target logic. I don’t believe we have that
>> concept at the moment define as part of the builder in SCons. Maybe that
>> should be added instead? I think that would solve the problem with A vs B
>> and make SCons generally more useful.
>>
>>
>>
>> Jason
>>
>>
>>
>>
>>
>>
>>
>> *From:* Scons-dev [mailto:scons-dev-bounces at scons.org] *On Behalf Of *William
>> Blevins
>> *Sent:* Wednesday, June 3, 2015 4:26 PM
>>
>> *To:* SCons developer list
>> *Subject:* Re: [Scons-dev] SCons cross-language support
>>
>>
>>
>>
>>
>> On Wed, Jun 3, 2015 at 4:53 PM, Kenny, Jason L <jason.l.kenny at intel.com>
>> wrote:
>>
>> I believe the copy of A is similar to linking on many systems. The linker
>> runs but it doesn’t have to as the import libs/so did not change, SCons
>> just assumes that if the dependent source changed everything above it must
>> be redone as well. I think this was a fix made back in the 0.98 days as at
>> one point this did not happen. I recall finding the code in the node that
>> controlled this. So for me this is not a scanner issue but a node issue in
>> that an actions should only go off if the sources are really different ( or
>> the function that says it different is true). In this case A.h still would
>> copy as B changed, but if we had a alpha.h that depends on A.h, it would
>> not copy as A.h MD5 was not different. Currently SCons would copy the
>> alpha.h. Such a change for example would also allow Parts which uses
>> hardlinks by default to be a lot faster as I would not have scons
>> rebuilding hard links redundantly when coping files.
>>
>>
>>
>> I agree here.  Install builder targets do not have binfo objects or at
>> least not MD5sum components.  This is a bug IMHO.  There are several
>> related tigris issues if I remember correctly.  Not sure about Copy targets.
>>
>>
>>
>> I am unclear on how removing case B would make A.h with the current patch
>> not copy A.h is there some difference in a node being different because it
>> is an implicit dependency ( ie scanner) vs that of being a source to a
>> builder?
>>
>>
>>
>> I would add a source_scanner to the Install builder that was
>> non-recursive, which would then break Case B, since it expects recursive
>> scanning of Install targets. A quick examination of the test case I
>> mentioned in the patch comments may be easier to understand than plain text
>> which regard to Case B expectations.
>>
>>
>>
>> What is the difference in the use case for Install vs copy? My view was
>> that Install is just a copy that adds the items to the known installed
>> items for packaging calls later.
>>
>>
>>
>> I'm not 100% sure here.  I think they both call copy_func, but their
>> setup is slightly different.  I just imagine that Install and Copy have
>> different semantic meanings.  Install is like a Copy + forget whereas Copy
>> should track the target as a full node.
>>
>>
>>
>> Either way I agree, if not for different reason, the use case B should
>> not be support.
>>
>>
>>
>> If I understand your vote correctly, you want Install targets
>> non-recursive; thus, fixing Case A and removing test coverage of Case B.
>>
>>
>>
>> Jason
>>
>>
>>
>> *From:* Scons-dev [mailto:scons-dev-bounces at scons.org] *On Behalf Of *William
>> Blevins
>> *Sent:* Wednesday, June 3, 2015 12:09 PM
>>
>>
>> *To:* SCons developer list
>> *Subject:* Re: [Scons-dev] SCons cross-language support
>>
>>
>>
>> To be clear, Case A and Case B are related.
>>
>>
>>
>> I can resolve one issue but not both, so its really a multiple choice
>> question.
>>
>> On Wed, Jun 3, 2015 at 9:47 AM, Kenny, Jason L <jason.l.kenny at intel.com>
>> wrote:
>>
>> My take is that the scanner changes should improve the ability for the
>> scanner to find files, anything short of that is a breaking backward
>> compatibility. Any change that fixes the ability to find files where before
>> it did not find them ( and it should have) if the correct direction. Test
>> cases that fail because the scanner is finding files better, when before it
>> did not find then and failed ( ie the test expects a failure)  is not a
>> regression. I believe this would say the B should not be supported as it
>> seems to be based on the test failing to pass because the scanner failed to
>> find files. When we do this fix and it find files, it works correctly ( ie
>> passes) which I would argue is not a bad thing.
>>
>>
>>
>> What I was stating below was that the main reason scanner failed for me
>> was because bad ordering with subst() caused the scanner to fail ( recusive
>> or not) However that is technically a different issue from this
>> improvement. I thought might be related, but it is not for the test cases
>> in question. My bad ..
>>
>>
>>
>> Given what I understand I agree with your suggestion to continue support
>> A case and drop  B support.
>>
>>
>>
>> One question …
>>
>> Case A: is this correct?
>>
>> We have A.h -> B.h
>>
>> The copy action for A.h is to copy as a.h.l
>>
>> The copy action for B.h is to copy as b.h.l
>>
>> There is a scanner for *.h
>>
>> There is no scanner for *.l ??  (I don’t think that matters for this case
>> as there no actions with *.h.l files as source files to some other target)
>>
>>
>>
>> Actions chain should be like.
>>
>>
>>
>> 1)      Scons see A.h depends on B.h so it installs b.h as b.h.l, then
>> it installs A.h as a.h.l
>>
>> 2)      On rebuild everything is up to date
>>
>> 3)      On modify of b.h scons does 1) set of actions.
>>
>>
>>
>> Correct? I think that is efficient and correct.
>>
>>
>>
>> Correct.  On step three, both B.h and A.h will be reinstalled due to the
>> dependency chain even though A.h has not changed.  It is correct, but not
>> efficient.  I believe to fix the efficiency, Install (and copy) should
>> behave more like rsync than a plain copy.  The install of A.h did not
>> happen on step 3 before the scanner improvements.
>>
>>
>>
>> I can fix the inefficiency by removing support for Case B.  In my opinion
>> Case B shouldn't be supported anyway, since I do not think that Use Case
>> makes sense for the Install builder.  I could possibly see using Copy for
>> this Use Case but not Install.  The issue with some of the SCons tests are
>> that the tests define how the program behaved previously, but they do not
>> define how the program was intended to behave.  I'm not sure whether Case B
>> was supported intentionally or it just happened to work.
>>
>>
>>
>>
>>
>> I should be able to say this in Parts for example as get the same logic
>> as the scanner are implicit….
>>
>> …
>>
>> env.CCopyAs([‘a.h.l’,’b.h.l’’],[‘a.h’,’b.h’])
>>
>> …
>>
>>
>>
>> Since the scanner for the .h files will be run by the CCopy builder in
>> Parts for a given file, even though I don’t have a scanner for the source
>> or target files in the builder directly.
>>
>> I should be able add such a test to verify this behavior with the
>> implicit scanner on a random builder. Is this correct?
>>
>>
>>
>> Thanks Jason
>>
>>
>>
>> *From:* Scons-dev [mailto:scons-dev-bounces at scons.org] *On Behalf Of *William
>> Blevins
>> *Sent:* Tuesday, June 2, 2015 6:00 PM
>> *To:* SCons developer list
>> *Subject:* Re: [Scons-dev] SCons cross-language support
>>
>>
>>
>> Jason,
>>
>>
>>
>> If I understand this correctly. I have a similar issue in Parts. I found
>> that it was that the scanner did not expand the path correctly via not
>> calling subst() before it tried to access the variable. This caused
>> failures in correctly finding libs and or header on first pass builds. It
>> would also call possible rebuild on second pass or in some cases a false
>> rebuild as SCons thought the path changed.
>>
>>
>>
>> This isn't (too my understanding) related to the patch in question.
>>
>>
>>
>> However I might have missed something in terms of this patch as I did not
>> know the install builder called a scanner. ( you did mean the install.py
>> tool.. or was this something else?) the install tools as I understand does
>> not have a scanner mapped to it.
>>
>>
>>
>> All builders call a scanner or at least try to find a scanner to call
>> (whether implicitly or explicitly).  Sometimes this was simply handled as a
>> None case.  In order to finish the patch, I need to make a choice (Case A
>> or Case B).  Neither are 100% backwards compatible with previous behavior,
>> but both are reasonable I believe.
>>
>>
>>
>> V/R,
>>
>> William
>>
>>
>>
>>
>> _______________________________________________
>> Scons-dev mailing list
>> Scons-dev at scons.org
>> https://pairlist2.pair.net/mailman/listinfo/scons-dev
>>
>>
>>
>>
>> _______________________________________________
>> Scons-dev mailing list
>> Scons-dev at scons.org
>> https://pairlist2.pair.net/mailman/listinfo/scons-dev
>>
>>
>>
>>
>> _______________________________________________
>> 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/20150827/2b5fca58/attachment-0001.html>