[Scons-users] Bug in up_to_date for directory targets
Bill Deegan
bill at baddogconsulting.com
Wed Mar 22 11:44:52 EDT 2017
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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://pairlist4.pair.net/pipermail/scons-users/attachments/20170322/6ad38cec/attachment-0001.html>
More information about the Scons-users
mailing list