[Scons-users] clarification on Depends()
Bill Deegan
bill at baddogconsulting.com
Sun Jul 8 01:02:15 EDT 2018
Jason,
That's a lot to chew on.
Let me read it a few times.. ;)
One note. If the decider is set to Timestamp-MD5, then Node.changed() (your
link to:
https://github.com/SCons/scons/blob/master/src/engine/SCons/Node/__init__.py#L1409
) calls File.changed_timestamp_then_content(), if the timestamp hasn't
changed we don't re-MD5 the file but we are copying the csig.
This is the cause of bug 2980.
(which I am currently working on ).
The fact the query is actually changing the data was surprising to me,
though I understand why it was done, there is very likely a better way to
do this.
-Bill
On Sat, Jul 7, 2018 at 9:32 PM, Jason Kenny <dragon512 at live.com> wrote:
> Hi Bill,
>
>
>
> Sort of late .. so I will (try to) make this quick.
>
>
>
> I have a PR (https://github.com/SCons/scons/pull/3129 ) All the test
> failures that are not related to the PR. I don’t have the details, but a
> few of the failure looked like issues related to node issues.
>
>
>
> As far as the hidden bugs in the node code.
>
> There are a number of them such as
>
> https://github.com/SCons/scons/blob/master/src/engine/
> SCons/Node/FS.py#L783
>
> https://github.com/SCons/scons/blob/master/src/engine/
> SCons/Node/FS.py#L3034
>
> https://github.com/SCons/scons/blob/master/src/engine/
> SCons/Node/__init__.py#L640 ( this is the one I referred to before)
>
> (there are others)
>
>
>
> All these raise an AttributeError exception, based on the “pre-slot” logic
> of the item not existing. Given the Slots to save memory, the attribute
> already exists with the value None. So for example, unless some code such as
>
> https://github.com/SCons/scons/blob/master/src/engine/
> SCons/Node/__init__.py#L667 is called ( in the case of the
> https://github.com/SCons/scons/blob/master/src/engine/
> SCons/Node/__init__.py#L640) The SCons.Executor.Null will not be created
> as expected. Given that self.executor is None because it already exists via
> the __slots__ definition. I think most of these cases need to be changed to
> test for None. I think it would also be better to have code such as
> https://github.com/SCons/scons/blob/master/src/engine/
> SCons/Node/__init__.py#L667 setting the attribute to None, instead of
> deleting it would be faster and more consistent.
>
>
>
> Since I am here I should point out that I think we can get a small speed
> up if we change some function such as https://github.com/SCons/
> scons/blob/master/src/engine/SCons/Node/__init__.py#L1409 to return once
> we know the file changed. Iterating as it does is a waste of time once we
> know the file is changed. I believe we should return
> https://github.com/SCons/scons/blob/master/src/engine/
> SCons/Node/__init__.py#L1452 and https://github.com/SCons/
> scons/blob/master/src/engine/SCons/Node/__init__.py#L1457 for example
> instead of continuing to test for changes.
>
> As far as need. I stated it before in this thread. I have a few reasons
> for this:
>
>
>
> 1) There are cases in which the scanner does not add the implicit
> depends correctly. In this cases it is not always obvious or easy to know
> what the target file is one “should” call Depends() on. It is also more
> usable/clearer to for the user to say that the source file depends on the
> “missing” header/file, rather than having to say the header/file is a
> dependency of the target. This is more so that case when the source files
> may be used as an input the more than one builder to make different targets.
>
> 2) I have a use case in which I want to have a dynamic builder. In this
> cases, the builder does not know all the sources until some other targets
> are created. However, from a management point of view I have would like to
> have these targets be a dependent of a node. At this point I need to have
> this node to have a target only builder such as the one I would define in
> the SourceDepends() API I propose. This makes this API a generic/reusable
> method to bind to trees.
>
> 3) Most important to rehash .. is to point of that some builder, such
> as SharedLibrary() is a chain of builders. Being able to tag a target as a
> depends to a source if convenient way to get something to work quickly ( it
> is often not always correct.. but we live in the real world) SourceDepends
> would allow the users a usable way to connect chains of node when there is
> a source node in the middle and have it work via adding for the user a
> builder that does nothing but allow the taskmaster to chain nodes together
> for correct builder ordering when it is being built.
>
>
>
> The taskmaster logic looks correct. ( I think it needs more comments in
> the code). I propose the solution I did as the code stored and sets state
> based on the node having a builder. My original idea was to see if the
> source node thinks it is_up_to_date alone with the current test of has it
> changed since the last time the given target node was built. This would see
> the “depends” information that the Depends call would add. I found this
> does not work as SCons does not store the bdependsigs correctly with the
> bdepends in bsig object in the DB ( because the node does not have a
> builder…). Because of this adding the check means SCons will always view
> the source node out of date. My proposed solution corrects this by adding a
> “do nothing” builder. This allows everything to work as is.
>
>
>
> Jason
>
>
>
> *From:* Bill Deegan <bill at baddogconsulting.com>
> *Sent:* Saturday, July 7, 2018 10:33 PM
>
> *To:* Jason Kenny <dragon512 at live.com>
> *Cc:* SCons users mailing list <scons-users at scons.org>
> *Subject:* Re: [Scons-users] clarification on Depends()
>
>
>
> I'm not sure I understand why this is needed?
>
>
>
> Also, which tests are failing?
>
>
>
> What file are you pointing to with the self.executor logic? (URL pointing
> at github file and line probably the most useful)
>
>
>
> -Bill
>
>
>
> On Sat, Jul 7, 2018 at 1:44 PM, Jason Kenny <dragon512 at live.com> wrote:
>
>
>
> I have gone over the code logic in the taskmaster in detail and have most
> of it in my head again. Given what it is doing I would like to propose
> adding a new method
>
>
>
> SourceDepends(source,node)
>
>
>
> This would basically be like Depends, expect that if the source node does
> not have a builder, it adds a target only builder to the source node that
> does nothing. If it does have a builder this is no different than
> Depends(). The main difference is that if you use SourceDepends() on a node
> that later adds a builder to it, Scons will complain ( as it would normally
> give when a node with two builders being added to it). In such cases, the
> user should use Depends ( as this a real target node now)
>
>
>
> This will allow for the case that I keep seeing people hit. ie some case
> in which the scanner cannot understand that a file is implicit depends.
>
>
>
> Do you have any concerns about accepting a PR with this function for SCons?
>
>
>
> I will also try to address some of the __slots__ bugs I found in the node
> logic. I think this may be why some of the tests are failing at the moment.
>
>
>
> Jason
> ------------------------------
>
> *From:* Scons-users <scons-users-bounces at scons.org> on behalf of Jason
> Kenny <dragon512 at live.com>
> *Sent:* Friday, July 6, 2018 6:07 PM
>
>
> *To:* Bill Deegan
> *Cc:* SCons users mailing list
> *Subject:* Re: [Scons-users] clarification on Depends()
>
>
>
> I do believe I found a hidden bug that is a result of code moving to using
> slots.
>
> def get_executor(self, create=1):
>
> """Fetch the action executor for this node. Create one if
>
> there isn't already one, and requested to do so."""
>
> try:
>
> executor = self.executor <-- This is will be None. It will
> never raise and expectation
>
> except AttributeError:
>
> if not create:
>
> I believe this code needs to be changed to test for self.executor being
> None.
>
> Jason
>
>
> ------------------------------
>
> *From:* Jason Kenny <dragon512 at live.com>
> *Sent:* Friday, July 6, 2018 5:48 PM
> *To:* Bill Deegan
> *Cc:* SCons users mailing list
> *Subject:* Re: [Scons-users] clarification on Depends()
>
>
>
> I take that back. I had my logic somewhat backwards. Nodes without
> builders are always up-to-date. If it does have a builder and it not always
> built, then we care if it is up_to_date()
>
> Jason
> ------------------------------
>
> *From:* Jason Kenny <dragon512 at live.com>
> *Sent:* Friday, July 6, 2018 5:05 PM
> *To:* Bill Deegan
> *Cc:* SCons users mailing list
> *Subject:* Re: [Scons-users] clarification on Depends()
>
>
>
> Hi Bill,
>
> Getting a little time to look at this in detail...
>
> Going through the taskmaster and I see some logic that seems pointless?
>
> ie..
>
> is_up_to_date = not t.has_builder() or \
> (not t.always_build and t.is_up_to_date())
>
> I believe it should be
>
> is_up_to_date = not t.has_builder()
>
> The reason for this is that if the node does not have a build the
> t.is_up_to_date() function will return the node is up-to-date if the node
> does not have a builder as well. The only value on this line "always build"
> has if replaced with the fact that it can only build if there is a builder.
> If it has a builder it is always added to the self.out_of_date list.
>
> Still digging in this code more. I feel pretty sure about this case as
> Alias nodes always have a builder on them as well
>
> Jason
>
>
> ------------------------------
>
> *From:* Jason Kenny
> *Sent:* Friday, June 8, 2018 9:07 AM
> *To:* Bill Deegan
> *Cc:* SCons users mailing list
> *Subject:* RE: [Scons-users] clarification on Depends()
>
>
>
> I am looking at it. I have to look better at what is happening in the
> taskmaster code. However I keep getting distracted with other issue. So I
> have not been able to reply back yet
>
>
>
> Jason
>
>
>
> *From:* Bill Deegan <bill at baddogconsulting.com>
> *Sent:* Tuesday, June 5, 2018 9:37 PM
> *To:* Jason Kenny <dragon512 at live.com>
> *Cc:* SCons users mailing list <scons-users at scons.org>
> *Subject:* Re: [Scons-users] clarification on Depends()
>
>
>
> Did you look at the taskmaster trace file?
>
>
>
>
>
> On Tue, Jun 5, 2018 at 6:18 PM, Jason Kenny <dragon512 at live.com> wrote:
>
> Bill ,
>
>
>
> I don’t mean to offend with the “generally pointless”. ( I realized that I
> might have been a little harsh with that statement.. sorry)
>
>
>
> What I have found is that when Scons prints this:
>
>
>
> +-hello
>
> | +-hello.o
>
> | | +-hello.c
>
> | | | +-fake.txt
>
> | | +-/usr/bin/gcc
>
> | +-/usr/bin/gcc
>
>
>
> You would think that a change to fake.txt would go up the change and cause
> to stuff to rebuild. However this is only true if
>
>
>
> | | +-hello.c
>
> | | | +-fake.txt
>
>
>
> Happens because of a builder. If it is a Depends() like it is here.
> Nothing happens. User is confused.
>
>
>
> If this looks like:
>
>
>
> +-hello
>
> | +-hello.o
>
> | | +-hello.c
>
> | | +-fake.txt
>
> | | +-/usr/bin/gcc
>
> | +-/usr/bin/gcc
>
>
>
> The hello.o will rebuild. If hello.c was used in more than one builder as
> a sources I would have to Depends() each target that hello.c might be part
> of.
>
>
>
> I understand the example here is a .c file. But don’t get hung up on that.
> This is a general builder(target,source) issue. Fake.txt here could have
> been #included via some funny macro expansion that SCons scanners cannot
> deal with correctly ( as is common with boost as an example)
>
>
>
> Bill. I am confused as based on what you stated it seems my first example
> of:
>
>
>
> Depends(["hello.c")],["fake.txt"])
>
> Program("hello","hello.c")
>
>
>
> Should have rebuilt hello.o as hello.c should have been seen as out of
> data. What did I miss here?
>
>
>
> Jason
>
>
>
>
>
> *From:* Bill Deegan <bill at baddogconsulting.com>
> *Sent:* Tuesday, June 5, 2018 7:56 PM
> *To:* Jason Kenny <dragon512 at live.com>
> *Cc:* SCons users mailing list <scons-users at scons.org>
>
>
> *Subject:* Re: [Scons-users] clarification on Depends()
>
>
>
> Jason,
>
>
>
> I've been spending a lot of time in the decider/taskmaster logic lately.
>
> Depends do actually matter and are not "generally pointless"
>
> They're added to Node's bdepend list and depend_set which combined with
> sources and implicit dependencies are what is used to determine if the
> sources are out of date with respect to the target.
>
>
>
> -Bill
>
>
>
> On Tue, Jun 5, 2018 at 5:47 PM, Jason Kenny <dragon512 at live.com> wrote:
>
> I think of this more as a make rule.
>
>
>
> I want to say for whatever reason that if the fake.txt is out of date that
> mean hello.c is out of date.
>
>
>
> So something like:
>
>
>
> Fake.txt:
>
>
>
> Hello.c:fake.txt
>
>
>
> Hello.o:hello.c
>
> cc hello.c
>
>
>
> Here in make land a change to fake.txt will spawn all rules that have
> fake.txt as a source.
>
>
>
> From what I understand the Depend() is generally pointless as the
> execution logic only cares about implicit depends of the source via the
> scanner, and if the source has a builder. If it has a builder then we care
> about depends of the source node, as now it is technically a target in the
> other builder. I can get different behavior via one of three ways.
>
>
>
> 1. Make an empty do nothing builder to make a depends() as a source of
> the of the node
> 2. Change/add a different decider to say this node is up-to-date if
> and only if it and any depends are all up-to-date. Currently this only
> returns True if this node in question is up-to-date, not if anything it
> depends on is as well.
> 3. Change the executor to care about depends of a source even if it
> does not have a builder
>
>
>
> Honestly there are special cases in which this is useful. Most of the time
> the scanner is the way to go.
>
>
>
> Jason
>
>
>
> *From:* Bill Deegan <bill at baddogconsulting.com>
> *Sent:* Tuesday, June 5, 2018 7:25 PM
> *To:* SCons users mailing list <scons-users at scons.org>
> *Cc:* Jason Kenny <dragon512 at live.com>
> *Subject:* Re: [Scons-users] clarification on Depends()
>
>
>
>
>
>
>
> On Tue, Jun 5, 2018 at 3:27 PM, Marc Branchaud <marcnarc at xiplink.com>
> wrote:
>
> On 2018-06-05 12:05 AM, Jason Kenny wrote:
>
> HI,
>
> I just want to clarify behavior of Depends() in SCons.
>
> I have a small sample:
>
> Depends(["hello.c")],["fake.txt"])
>
> Program("hello","hello.c")
>
> If I run this sample “hello” will build. If I change fake.txt, nothing
> will rebuild.
>
>
> Right. This is because you have nothing that builds hello.c, so SCons
> doesn't care that hello.c depends on anything.
>
> But SCons does know how to build hello.o. That knowledge is implicit in
> the Program builder. So SCons respects
> Depends("hello.o", "fake.txt")
>
> Put another way, SCons only performs actions to build hello.o (and,
> ultimately, hello), so it only cares about dependencies for the actions it
> undertakes.
>
> You could also use the target returned by the Program builder:
> hello_target = Program("hello", "hello.c")
> Depends(hello_target, "fake.txt")
> This achieves the same thing, and lets you avoid knowing the Program
> builder's intermediate artifacts.
>
>
>
> It would rebuild hello, but not necessarily rebuild hello.o
>
>
>
> So not exactly the same thing.
>
>
>
> -Bill
>
>
>
>
> M.
>
> I have a tree like this:
>
> +-hello
>
> | +-hello.o
>
> | | +-hello.c
>
> | | +-fake.txt
>
> | | +-/usr/bin/gcc
>
> | +-/usr/bin/gcc
>
> If I change the sample to this ( depends() is not hello.o vs hello.c)
>
> Depends(["hello.c")],["fake.txt"])
>
> Program("hello","hello.c")
>
> Now when I change fake.txt hello.o will rebuild. The tree here looks like:
>
> +-hello
>
> | +-hello.o
>
> | | +-hello.c
>
> | | +-fake.txt
>
> | | +-/usr/bin/gcc
>
> | +-/usr/bin/gcc
>
> Is this right… I have to know the target of a builder to use? I cannot use
> the source of a builder?
>
> Jason
>
>
>
> _______________________________________________
> Scons-users mailing list
> Scons-users at scons.org
> https://pairlist4.pair.net/mailman/listinfo/scons-users
> <https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpairlist4.pair.net%2Fmailman%2Flistinfo%2Fscons-users&data=02%7C01%7C%7Ca25e1c3b08b94567e3c808d5cb43e88c%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636638414874226947&sdata=9xTM%2Fogjcekowt8cySadqFPfrk7mKGWBLzEwON2hL0U%3D&reserved=0>
>
> _______________________________________________
> Scons-users mailing list
> Scons-users at scons.org
> https://pairlist4.pair.net/mailman/listinfo/scons-users
> <https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpairlist4.pair.net%2Fmailman%2Flistinfo%2Fscons-users&data=02%7C01%7C%7Ca25e1c3b08b94567e3c808d5cb43e88c%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636638414874226947&sdata=9xTM%2Fogjcekowt8cySadqFPfrk7mKGWBLzEwON2hL0U%3D&reserved=0>
>
>
>
>
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://pairlist4.pair.net/pipermail/scons-users/attachments/20180707/7fdaaae7/attachment-0001.html>
More information about the Scons-users
mailing list