[Scons-dev] SCons cross-language support
William Blevins
wblevins001 at gmail.com
Wed Jun 3 19:31:53 EDT 2015
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/20150603/d423d170/attachment-0001.html>
More information about the Scons-dev
mailing list