[Scons-users] clarification on Depends()
Jason Kenny
dragon512 at live.com
Sun Jul 8 00:32:49 EDT 2018
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<mailto: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<mailto:scons-users-bounces at scons.org>> on behalf of Jason Kenny <dragon512 at live.com<mailto: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<mailto: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<mailto: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<mailto:bill at baddogconsulting.com>>
Sent: Tuesday, June 5, 2018 9:37 PM
To: Jason Kenny <dragon512 at live.com<mailto:dragon512 at live.com>>
Cc: SCons users mailing list <scons-users at scons.org<mailto: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<mailto: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<mailto:bill at baddogconsulting.com>>
Sent: Tuesday, June 5, 2018 7:56 PM
To: Jason Kenny <dragon512 at live.com<mailto:dragon512 at live.com>>
Cc: SCons users mailing list <scons-users at scons.org<mailto: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<mailto: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<mailto:bill at baddogconsulting.com>>
Sent: Tuesday, June 5, 2018 7:25 PM
To: SCons users mailing list <scons-users at scons.org<mailto:scons-users at scons.org>>
Cc: Jason Kenny <dragon512 at live.com<mailto: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<mailto: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<mailto: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<mailto: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/20180708/4ba2907e/attachment-0001.html>
More information about the Scons-users
mailing list