[Scons-dev] SCons cross-language support

[Kenny, Jason L]

<< 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)

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<mailto: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<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<mailto: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<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<mailto:Scons-dev at scons.org>
https://pairlist2.pair.net/mailman/listinfo/scons-dev


_______________________________________________
Scons-dev mailing list
Scons-dev at scons.org<mailto: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/20150603/51baca22/attachment-0001.html>