[Scons-users] Bug in up_to_date for directory targets

Manish Vachharajani manishv at unbounded.systems
Tue Mar 21 19:17:15 EDT 2017 

Exactly, so now we are on the same page.

I don't believe this has ever worked.  When looking through the debugger
and the code, I vaguely recall encountering and working around this same
problem 4-5 years ago, and then had forgotten about it.  Send me a private
email if you want me to walk you through what I found in the code.

Unfortunately, I do not think the fix is simple since it will change how
SCons calculates readiness for everything, which is why I didn't submit a
patch.  In particular, I think that the way is_up_to_date works with the
SCons.Node.changed method is the issue - the is_up_to_date for file
*targets* is the thing that looks at the signatures of its children in the
dependence tree, even though you'd think that is_up_to_date on a node would
check its own signature (which it does not).  The is_up_to_date function on
Dir nodes just iterates over its children in the tree and calls
is_up_to_date which the Dir Node is expecting to check the signature (as
one would expect given the method name) which means nothing ever checks the
signature if you only have Dir targets.  If the directory doesn't exist
though, the target is automatically considered out of date which is why the
mkdir examples always seem ok.

The real fix is to fix is_up_to_date for File nodes, in my opinion, but
this has massive implications.  The hacky, but more limited fix, is to
modify is_up_to_date on Dir nodes to check the signature for its children
just like the File node does.   The problem with the hacky fix is that
builders that us is_up_to_date (like our more complex npm builder) then
have to be aware of this issue and work around it as well.  Oh, and this
analysis is based on my very limited inspection of the code, so I could
also be way off here.

Manish

On Tue, Mar 21, 2017 at 3:39 PM, William Blevins <wblevins001 at gmail.com>
wrote:

> I see what you are saying. I need to look into it some. Did this work in a
> previous version of SCons? What version did it stop working in? What
> version of Python are you using?
>
> It's a bit strange. I needed to fix your example, but I got it to
> replicate. At a glance, the dependency tree looks correct, but nothing
> happens when the permissions file is updated.
>
> V/R,
> William
>
> On Tue, Mar 21, 2017 at 2:09 PM, Manish Vachharajani <
> manishv at unbounded.systems> wrote:
>
>> Let me try to get the discussion back to my main point.  There is a bug
>> in scons and there should be a ticket for it and a fix (unless I've
>> misunderstood something, which from the discussion I don't think I have).
>> The nature of the bug is this:
>>
>> 1) The user tells scons that there is a target (foo in my example)
>> 2) The target depends on a source (foo-contents.txt in my example)
>> 3) When the target is out of date with respect to the source, rerun the
>> builder
>>
>> It isn't scons' job, in my opinion, to second guess me and decide that it
>> doesn't really need to run the builder, or that the target isn't the
>> precise output set of the builder etc.  The point is that there is a target
>> which is out of date with respect to the source and scons is not running
>> the builder.  Everything else, IMHO, is an aside.  Does that make it
>> clear?  I'm not trying to be rude but something is getting lost when I'm
>> trying to be indirect.
>>
>> I'm glad to discuss what I believe is the root cause based on my
>> investigations in the python debugger.  99% of the time this doesn't matter
>> because there is almost never a directory only set of targets with a
>> builder that isn't simply a mkdir builder or equivalent.  Note, however, if
>> the builder were something like:
>>
>> env.Command(['/a/b/c'],
>>                          ['permissions.txt'],
>>                          [mkdir $TARGET,
>>                           chmod `permissions.txt` $TARGET])
>>
>> scons would fail to update the permissions on the directory when
>> permissions.txt changed if /a/b/c already existed.  I hope that makes it
>> more clear the exact nature of the issue.  My example is simple because it
>> is the smallest SConstruct I could create to demonstrate the issue.
>>
>>  For my specific case, I have workarounds, and a custom builder that uses
>> them.  A custom builder doesn't help without the workarounds because the
>> problem is in the up to date logic, not the scanner and action logic in
>> scons.  Oh, and while package managers let you see content, it doesn't help
>> if the package is going to fetched from the network and I need to calculate
>> dependencies before the package is fetched.
>>
>> Manish
>>
>>
>>
>> On Tue, Mar 21, 2017 at 10:53 AM, William Blevins <wblevins001 at gmail.com>
>> wrote:
>>
>>> Manish,
>>>
>>> Ah, ok. I was confused when your original example seemed so simple. You
>>> should probably create a custom builder then:
>>> https://bitbucket.org/scons/scons/wiki/ToolsForFools
>>>
>>> It isn't a bug that SCons doesn't rebuild targets it doesn't have. The
>>> issue is that you need to tell SCons about said targets. Do this via a
>>> custom builder.
>>>
>>> I haven't used npm, but most package managers have a way to list the
>>> contents of a package, so just convert that list into a target set. I would
>>> need more information about the source -> target dependency chain in terms
>>> of actual vs expected in order to say much more.
>>>
>>> Sorry for the fix response and run, but normally I don't get time to
>>> answer emails this early in the day. Let us know if you think a custom
>>> builder will not solve your problem.
>>>
>>> V/R,
>>> William
>>>
>>> On Tue, Mar 21, 2017 at 12:22 PM, Manish Vachharajani <
>>> manishv at unbounded.systems> wrote:
>>>
>>>> If you look at my original message, the problem is that in my real use
>>>> case, I have no idea what the file targets will be.  The only target I know
>>>> will be built by the builder is a set of directories.  Your example simply
>>>> causes scons to have a file target in the second Command builder which
>>>> works around the bug as I describe in my original email.  In real life, my
>>>> builder looks something like this;
>>>>
>>>> env.Command(['node_modules/minimist', 'node_modules/@types/node, ...],
>>>> #these are directories
>>>>             ['package.json'],
>>>>             ['npm install --global-style'],
>>>>             chdir=Dir('.')) #chdir because npm doesn't have a way to
>>>> install elsewhere
>>>>
>>>> In this case, I have no idea what files npm install will create.  The
>>>> targets array is actually created by reading package.json and looking at
>>>> the dependencies and devDependencies fields of the main object, so those
>>>> aren't even hard-coded in the SConstruct/SConscript.
>>>>
>>>> Again, if I fake out scons by have a 'node_modules/.built' File target
>>>> in the list and add a 'touch node_modules/.built' command to the builder
>>>> after 'npm install', all will work as expected because there is a File
>>>> target.  In your example, things work because the second builder has a file
>>>> target and SCons will only run the first builder in your example when foo
>>>> does not exist, regardless of the state of foo-contents.txt in relation to
>>>> what is in .sconsign.  That is the essence of the bug, if the target
>>>> directory exists and it is the only target, the builder will not run
>>>> regardless of the state of the sources.
>>>>
>>>> Manish
>>>>
>>>>
>>>> On Mon, Mar 20, 2017 at 9:09 PM, William Blevins <wblevins001 at gmail.com
>>>> > wrote:
>>>>
>>>>> Manish,
>>>>>
>>>>> * Why does my example not work?
>>>>>
>>>>> SCons Comand Builder parameters are target(s), source(s), command(s):
>>>>> http://scons.org/doc/HTML/scons-user.html#chap-builders-commands
>>>>>
>>>>> Foo is the target parameter, and foo-contents.txt is the source
>>>>> parameter. Targets explicitly depend on sources. In your example, directory
>>>>> foo depends on source file foo-contents.txt; target file copy does not
>>>>> depend on source file.
>>>>>
>>>>> * How do I fix this issue?
>>>>>
>>>>> 1. Call Command Builder correctly: Command('foo', 'foo-contents',
>>>>> 'mkdir $TARGET') which creates file dependency on directory, and then
>>>>> Command('foo/foo-contents.txt', 'foo-contents.txt', 'cp $SOURCE
>>>>> $TARGET') which creates file target dependency on file source.
>>>>>
>>>>> 2: [PREFERRED] Use the SCons FileSystem factory
>>>>> http://scons.org/doc/HTML/scons-user.html#chap-factories
>>>>>
>>>>> V/R,
>>>>> William
>>>>>
>>>>>
>>>>> On Mon, Mar 20, 2017 at 1:47 PM, Manish Vachharajani <
>>>>> manishv at unbounded.systems> wrote:
>>>>>
>>>>>> I believe there is a bug in how up to date is computed for file nodes
>>>>>> and directory targets.  Below is a SConstruct and run output illustrating
>>>>>> the bug.
>>>>>>
>>>>>> ##############
>>>>>> # SConstruct
>>>>>> env = Environment()
>>>>>> targets = env.Command('foo',
>>>>>>                       'foo-contents.txt',
>>>>>>                       ['mkdir foo',
>>>>>>                        'cp foo-contents.txt foo'])
>>>>>> env.Default(targets)
>>>>>>
>>>>>> ##############
>>>>>> # Shell log
>>>>>>
>>>>>> # Create the file that will be installed into foo/foo-contents.txt
>>>>>> $ echo "Hello, " >> foo-contents.txt
>>>>>> # All is well, scons runs the command builder as expected
>>>>>> $ scons
>>>>>>
>>>>>> scons: Reading SConscript files ...
>>>>>> scons: done reading SConscript files.
>>>>>> scons: Building targets ...
>>>>>> mkdir foo
>>>>>> cp foo-contents.txt foo
>>>>>> scons: done building targets.
>>>>>>
>>>>>> # All is well, the target is up to date as expected
>>>>>> $ scons
>>>>>>
>>>>>> scons: Reading SConscript files ...
>>>>>> scons: done reading SConscript files.
>>>>>> scons: Building targets ...
>>>>>> scons: `foo' is up to date.
>>>>>> scons: done building targets.
>>>>>>
>>>>>> # Uh-oh, source is updated but the target is considered up to date!
>>>>>> $ echo "World!" >> foo-contents.txt
>>>>>> $ scons
>>>>>>
>>>>>> scons: Reading SConscript files ...
>>>>>> scons: done reading SConscript files.
>>>>>> scons: Building targets ...
>>>>>> scons: `foo' is up to date.
>>>>>> scons: done building targets.
>>>>>>
>>>>>> For the bug to trigger, all targets for a builder must be
>>>>>> directories.  The target directory must exist and the source file must
>>>>>> exist and be out of date due to a content update (resulting in a signature
>>>>>> mismatch).
>>>>>>
>>>>>> I think the problem is that is_up_to_date for File nodes only checks
>>>>>> the signatures of its children, which avoids this bug as long as one of the
>>>>>> builder targets is a File and not a Directory.  However, I haven't done
>>>>>> enough analysis to be sure that this is the issue.
>>>>>>
>>>>>> A work around is to make sure that you have a file (or create a dummy
>>>>>> .built file) and include it as a target.
>>>>>>
>>>>>> This is showing up specifically for us when trying to calculate
>>>>>> dependencies for a package manager (npm) where we have no insight into the
>>>>>> contents of the resulting directory.
>>>>>>
>>>>>> Manish
>>>>>>
>>>>>> _______________________________________________
>>>>>> 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
>>>>>
>>>>>
>>>>
>>>> _______________________________________________
>>>> 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
>>>
>>>
>>
>> _______________________________________________
>> 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/20170321/2dffe5c2/attachment-0001.html>

More information about the Scons-users mailing list