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

Fri Aug 3 18:12:46 EDT 2018

On Fri, Aug 3, 2018 at 2:16 PM, Matthew Marinets <
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) 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 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)" (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> On Behalf Of Alistair
> Buxton
> Sent: August 3, 2018 12:52
> To: SCons users mailing list <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> 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
> _______________________________________________
> Scons-users mailing list
> Scons-users at scons.org
> https://pairlist4.pair.net/mailman/listinfo/scons-users
> _______________________________________________
> Scons-users mailing list
> 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/20180803/93fd4eaa/attachment-0001.html>