[Scons-users] Bug in up_to_date for directory targets
Bill Deegan
bill at baddogconsulting.com
Wed Mar 22 13:15:14 EDT 2017
There're 1400+ tests.
There's a good testing guide in the wiki.
You'll probably benefit from taking a look at the developers guide there as
well.
-Bill
On Wed, Mar 22, 2017 at 12:32 PM, Manish Vachharajani <
manishv at unbounded.systems> wrote:
> 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
>>
>>
>
> _______________________________________________
> 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/2558f68e/attachment-0001.html>
More information about the Scons-users
mailing list