[Scons-users] Recursive directories, and the ignoring of filenames.

Fri Aug 3 20:35:34 EDT 2018

Sorry about jumping to conclusions about how it should work. I was just a bit excited because I finally managed to reproduce the bug after a couple hours on and off trying and wasn’t expecting it to work that way.

So to summarize:
-We can change the behaviour of a builder to rebuild when source names change with:

env.SomeBuilder(target, sources)
env.Depends(target, Value([str(x) for x in sources]))

-Having to do this manually gives more flexibility; “str(x)” can also be “x.name” or some kind of relative path function.
-Having similar functionality built into the decider is in the works, but is more complicated than it seems
-I should be using string paths instead of Dir / File objects in my scripts (?)

I wasn’t too familiar with Value() nodes, so it seems I jumped to conclusions.
(also don’t mind me forgetting to hit the send button two hours ago)

Have a good weekend everyone! : D

-Matthew

From: Scons-users <scons-users-bounces at scons.org> On Behalf Of Bill Deegan
Sent: August 3, 2018 15:13
To: SCons users mailing list <scons-users at scons.org>
Subject: Re: [Scons-users] Recursive directories, and the ignoring of filenames.

On Fri, Aug 3, 2018 at 2:16 PM, Matthew Marinets <Matthew.Marinets at kardium.com<mailto:Matthew.Marinets at kardium.com>> wrote:
I have reproduced the issue, and it's definitely a bug. It's very specific, which is probably why no one else has run into it before.

This is very odd SConstruct..

My code:
# SConstruct
env = Environment(ENV=os.environ)
src = Dir('src')
build = Dir('build')

sources = Glob(pattern = str(src) + '/*.c')
print([str(x) for x in sources])

# Copy only the first source, but give the builder all sources
output = env.Command(target = [build.File(x.name<http://x.name>) for x in sources[:1]], source = sources, action = 'cp $SOURCE $TARGET.dir')

# Here's how I'd write it. More SCons'ian
import os
env = Environment(ENV=os.environ)
src = 'src'
build = 'build'

sources = Glob(src + '/*.txt')

sources_string_list=[str(x) for x in sources]
print(sources_string_list)

# Copy only the first source, but give the builder all sources
output=env.Command("${SOURCES[0].file}", sources, 'cp $SOURCE ${TARGET.dir}')

# The line below makes the command rerun when the list of filenames with path changes.
# Note: No need to change decider or do any magic here..
env.Depends(output, Value(sources_string_list))

------
with the same directory structure as before. (src/a.c, src/b.c, src/qwer.c)

-Running the build once copies a.c to build, as expected. Running the build again says that build/a.c is up to date.
-Making a change to any source (a.c, b.c, or qwer.c) causes a rebuild
-Changing the name of qwer.c to, say q.c DOES NOT CAUSE A REBUILD. This is the bug.

Is it though?
So target a depends on c,d,e,f
If none of the contents change in the files it depends on then does it really need to be rebuilt?
(at least for a standard compilation model)

If the order changes, then it does need to be rebuilt as this can have implications for the contents of the target (in a compilation model)
Certainly with make this would not trigger a rebuild (assuming the timestamp didn't change).
If you want SCons to check timestamp first and then (if it's changed) check content then you'd use MD5-timestamp decider.

We would expect that if changing the contents of qwer.c causes a rebuild, then changing the name of qwer.c to something else should also cause a rebuild.
Additionally, adding an env.Depend() DOES NOT WORK. This has to do with how SCons handles its dependency checking:

Actually it's going to depend on what type of Node(s) you're adding via Depends.. It's method to determine "changed" will be called.

The method in charge of determining if a node is out of date comes from SCons.Node.__init__.py, in class Node, method changed(). The method checks three things:
-If the length of the source list has changed
-If the decider decides that a child is out of date.
-If the build action changed

Not entirely correct. The items are Nodes and can be many types. Each type can implement their own changed functions, which function is called is based o the type and also on the decider set in the Environment (or as a default for all environments if the Environment's hasn't been explicitly set)

The only potential place that can check if a child's name changed is the decider, and the default SCons decider doesn't. It merely compares item x in the current source list with item x in the old source list, and runs hashes only on the contents. I'm still not too great at navigating the SCons codebase, but I think it comes back to SCons.Node.changed_content(), which checks the csig attribute. Csig in turn comes from get_csig(), which only runs MD5 on a node's get_contents() output.

Which is valid in the traditional compilation model where such files would also be listed on the command line, the ordering the files are evaluated is deterministic so assuming dependencies come from the same "place" they will be in the same order and thus comparing old vs new is valid

**The script-side temporary fix would have to adjust the decider somehow.
-This function needs to take three arguments. From the SCons source code, it looks like "decider(dependency, target, prev_ni)" is used
-This function should return True if the node changed, false if it did not.
-A user could reasonably copy functions changed_content() and get_csig() from SCons/Node.FS.py<http://Node.FS.py> and use those as a basis for a new decider.
-The adjusted decider would simply append a node's name to its contents when if calculates a csig.
I'm not 100% exactly how to implement this myself (I'm no hashing expert), so I would appreciate if anyone with the requisite know-how could implement this and share the code.

**The full SCons-side fix is probably going to include a node's name when getting a hash.
-In SCons.Node.get_csig(), we would need to change the "csig = SCons.Util.MD5signature(contents)" to  "csig = SCons.Util.MD5signature(contents + self.name<http://self.name>)" (might have type issues)
-Either adjust get_content_hash() to include the node's name, or create a new method get_node_hash() that includes the node's name.

I'm deep in the code on a fix for MD5-timestamp which does pretty much this.
Note that all seems simple(ish) until you start adding Repository() and variant dirs and cachedirs
In these cases you may do a build which pulls a built object from a repository, or from a variant dir where the actual path differs but the requested path remains the same and so may or may not match the previous builds actual path, but may be valid and not require a rebuild.

When thinking about handling out-of-date ness during builds some things to keep in mind.
A) the goal when SCons was created (based on Cons) was to be better than Make.
B) Make doesn't care about the list of files, the content of the files, or the command line, only if the listed source(s) are newer than the listed target(s)

Hope my constant walls of text aren't wearing on people : P

Nope. I appreciate well though out discussion rather than walls of text based on assumptions of how SCons "should" work.

-Matthew

-----Original Message-----
From: Scons-users <scons-users-bounces at scons.org<mailto:scons-users-bounces at scons.org>> On Behalf Of Alistair Buxton
Sent: August 3, 2018 12:52
To: SCons users mailing list <scons-users at scons.org<mailto:scons-users at scons.org>>
Subject: Re: [Scons-users] Recursive directories, and the ignoring of filenames.
On 3 August 2018 at 20:41, Bill Deegan <bill at baddogconsulting.com<mailto:bill at baddogconsulting.com>> wrote:

>> Because the purpose of the example was to demonstrate that scons does
>> not reliably rebuild when the sources list changes. The fact that it
>> rebuilds if the action string changes is completely irrelevant, and
>> not helpful in the case where the tool takes a single directory as
>> the only argument.
>
>
> So if this causes the appropriate behavior but for the wrong "reason",
> then you will not use it?

I cannot use it because the tool in question only accepts a single directory as the sole argument, and so I could not list all the files in the action string even if I wanted to. (Which I do not.)

>
> As I understand it, you're trying to solve two issues to get the build
> you
> want:
> 1) Find the list of all dependencies correctly.  The scanner can
> resolve this.  (Especially if some of the files are built files, which
> you've yet to answer from a previous response in this email thread)
> 2) Decide if you need to rebuild (For this to work you need a correct
> list of dependencies, once again the scanner can solve this part of the puzzle).
> Then you just need an appropriate decider.

No, this is incorrect on both counts.

1) I don't need the list of dependencies, because the tool I am running only accepts a single directory as the sole argument. The list of dependencies is therefore useless for the purposes of generating the action string.

2) Having a correct list of dependencies does not help in deciding if I need to rebuild, because scons will not check if the filenames have changed. It will therefore give the wrong result when given a correct list of dependencies. This is the issue I am reporting, and is the reason why none of the suggestions made so far in this thread will work, except for the Value() based workaround I initially presented.

--
Alistair Buxton
a.j.buxton at gmail.com<mailto:a.j.buxton at gmail.com>
_______________________________________________
Scons-users mailing list
Scons-users at scons.org<mailto:Scons-users at scons.org>
https://pairlist4.pair.net/mailman/listinfo/scons-users
_______________________________________________
Scons-users mailing list
Scons-users at scons.org<mailto:Scons-users at scons.org>
https://pairlist4.pair.net/mailman/listinfo/scons-users

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://pairlist4.pair.net/pipermail/scons-users/attachments/20180804/89c7c707/attachment-0001.html>