[Scons-users] Bug in up_to_date for directory targets

Manish Vachharajani manishv at unbounded.systems
Wed Mar 22 00:57:20 EDT 2017 

The only work around I've found is to have a non-directory target, either a
dummy file that you have to create in the builder or a file you know will
be created by the builder.

A more narrow fix would be to update the is_up_to_date code for directories
to check the signatures of its children in the dependence tree explicitly.
This would fix the issue with Dir targets, leave other target types alone,
and only change the behavior of is_up_to_date for Dir nodes for the better,
in my opinion.  I'm not sure how far reaching this narrower fix would be.

Manish

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

> Value node doesn't seem to work. I don't think values can be "built," so
> that isn't a reasonable route.
>
> V/R,
> William
>
> On Wed, Mar 22, 2017 at 12:26 AM, William Blevins <wblevins001 at gmail.com>
> wrote:
>
>> Manish,
>>
>> You conclusion about this relating to a directory node specifically seems
>> to be accurate. A brief chat with some individuals on the developer list
>> seemed to confirm this. I assume this never behaved in the manner you
>> desire. The question is, as you have already stated, what are the
>> implications for this change? Under 99% of circumstances, you just want to
>> make the directory if it doesn't exist.
>>
>> One possible work around idea was to use a value node as the source, but
>> I haven't tried this myself.
>>
>> V/R,
>> William
>>
>> On Tue, Mar 21, 2017 at 7:17 PM, Manish Vachharajani <
>> manishv at unbounded.systems> wrote:
>>
>>> 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/scon
>>>>>>>> s-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
>>>>
>>>>
>>>
>>> _______________________________________________
>>> 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/b757c463/attachment-0001.html>

More information about the Scons-users mailing list