[Scons-users] Bug in up_to_date for directory targets

Manish Vachharajani manishv at unbounded.systems
Wed Mar 22 12:32:00 EDT 2017 

I can give it a shot, though it will take a while to run proper tests since
it isn't a top priority.  Are there a good set of unit tests for scons?

Manish

On Wed, Mar 22, 2017 at 9:44 AM, Bill Deegan <bill at baddogconsulting.com>
wrote:

> Manish,
>
> If you're up for attempting a fix (and hopefully  some tests), we'll work
> to get it in.
> Be aware there's a big push to get py2 + py3 in default branch at the
> moment.
>
> Thanks,
> Bill
> Co-Manager, SCons Project
>
> On Wed, Mar 22, 2017 at 12:57 AM, Manish Vachharajani <
> manishv at unbounded.systems> wrote:
>
>> 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
>>>
>>>
>>
>> _______________________________________________
>> 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/20170322/572a81f4/attachment-0001.html>

More information about the Scons-users mailing list