On Wed, Jun 3, 2015 at 4:53 PM, Kenny, Jason L <[email protected]> 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:[email protected]] *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 <[email protected]> > 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:[email protected]] *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 > [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
