[Scons-users] Possible bug with cache and Value()
Bill Deegan
bill at baddogconsulting.com
Wed Nov 22 15:32:27 EST 2017
Tim,
Yes, it's not a proper fix, but a workaround.
Currently get_cachedir_csig() for FS nodes is returning strings.
So when mixed with bytes from the Python Value() nodes, in the list which
is being joined it throws a TypeError.
So as I said, it's wrong (in that get_content() and signatures from such
should always be bytes).
But making that change will allow him to use py3 with scons 3.0.1 until a
proper fix is available.
And, I would not expect it to produce incorrect results unless a value
node's contents are not unicode encodeable.
Rob's issue does point out two things:
1) There is no test covering this case
2) content signatures aren't always bytes as I would expect, and as they
should be.
Would you like to take whack at sorting this issue out?
I'm working on performance improvements for 3.1 (or 4.0 depending if such
changes break compatability) and could us such time as it would take to
address this to move the ball forward on performance if someone else can
take it.. ;)
-Bill
On Wed, Nov 22, 2017 at 11:47 AM, Tim Jenness <tjenness at lsst.org> wrote:
> The get_text_contents() vs get_contents() code seems to be an interesting
> part of the port to python3. I’m not clear why the get_text_contents()
> implementation in the lines above this is not using
> kid.get_text_contents(). It seems strange to me that it getting the bytes
> and then decoding them to strings (without really knowing how to decode)
> and not asking for the strings directly.
>
> The fix below in get_contents seems wrong to me. get_contents is defined
> to return bytes and get_text_contents is defined to return characters, so
> having the code ever return the contents of get_text_contents() is always
> going to be wrong so that except clause must be a bug on python3. We had a
> similar problem in the alpha release of Node/FS.py where get_text_contents
> was returning bytes in the except clause.
>
> As for the reported bug, isn’t the problem below caused by the join being
> ‘ ,’.join() and not b’ ,’.join()? If sigs are meant to be bytes?
>
>
> On Nov 22, 2017, at 12:34 , Bill Deegan <bill at baddogconsulting.com> wrote:
>
> Try changing the following:
> SCons/Node/Python.py around line 139:
>
> def get_contents(self):
> text_contents = self.get_text_contents()
> try:
> return text_contents
> except UnicodeDecodeError:
> # Already encoded as python2 str are bytes
> return text_contents
>
>
>
> The return text_contents is orignally test_contents.encode(), which
> changes it to bytes.
> Then later the code tries to join a str space with a byte string and
> crashes.
>
> I'm going to say this is not actually correct for python 3, but it will
> work and shouldn't have any issues.
> The reason I'm saying it's not correct is because I think all signitures
> should be bytes and not strings.
> For py2.7 it doesn't matter.
> For Py 3.5+ I shouldn't matter, but I'm trying to keep all signatures as
> bytes and be calculated that way. In the case that the Value() node had a
> byte string value which wasn't encodable as a unicode string I would expect
> it to break (read stack trace).
>
>
> Thoughts?
>
> -Bill
>
>
> On Wed, Nov 22, 2017 at 7:18 AM, Bill Deegan <bill at baddogconsulting.com>
> wrote:
> >
> > Rob,
> >
> > At first glance looks like a bug on a code path which must not be
> covered by our tests.
> > string vs bytes is a dead giveaway.
> >
> > You must be running scons 3.0.1 with python 3.5+?
> > Can you try running with python 2.7.x? (It should work)
> >
> > -Bill
> >
> > On Wed, Nov 22, 2017 at 3:36 AM, Rob Spanton <rob at robspanton.com> wrote:
> >>
> >> Hi,
> >>
> >> I recently upgraded to scons 3.0.1, and I'm encountering a problem with
> using
> >> CacheDir() in combination with Value(). I can replicate the issue with
> this
> >> minimal SConstruct:
> >>
> >> CacheDir("cache")
> >> Command("somefile", Value("test"), "echo hi > $TARGET")
> >>
> >> I've included the traceback that this produces at the end of this
> email. I
> >> think what I'm doing with CacheDir, Command and Value is legitimate
> but am
> >> entirely happy to be persuaded that it is not!
> >>
> >> I can work with this for the moment by disabling the cache, but it
> would be
> >> useful if someone with some more in-depth scons knowledge than me
> could shed
> >> some light on what's going on.
> >>
> >> Thanks,
> >>
> >> Rob Spanton
> >>
> >> ---
> >>
> >> scons: Reading SConscript files ...
> >> scons: done reading SConscript files.
> >> scons: Building targets ...
> >> scons: *** [somefile] TypeError : sequence item 0: expected str
> instance, bytes found
> >> Traceback (most recent call last):
> >> File "/usr/lib/scons/SCons/Action.py", line 689, in __call__
> >> cmd = self.strfunction(target, source, env, executor)
> >> TypeError: CacheRetrieveString() takes 3 positional arguments but 4
> were given
> >>
> >> During handling of the above exception, another exception occurred:
> >>
> >> Traceback (most recent call last):
> >> File "/usr/lib/scons/SCons/Taskmaster.py", line 241, in execute
> >> if not t.retrieve_from_cache():
> >> File "/usr/lib/scons/SCons/Node/FS.py", line 2925, in
> retrieve_from_cache
> >> return self.get_build_env().get_CacheDir().retrieve(self)
> >> File "/usr/lib/scons/SCons/CacheDir.py", line 263, in retrieve
> >> if CacheRetrieve(node, [], env, execute=1) == 0:
> >> File "/usr/lib/scons/SCons/Action.py", line 691, in __call__
> >> cmd = self.strfunction(target, source, env)
> >> File "/usr/lib/scons/SCons/CacheDir.py", line 66, in
> CacheRetrieveString
> >> cachedir, cachefile = cd.cachepath(t)
> >> File "/usr/lib/scons/SCons/CacheDir.py", line 224, in cachepath
> >> sig = node.get_cachedir_bsig()
> >> File "/usr/lib/scons/SCons/Node/FS.py", line 3391, in
> get_cachedir_bsig
> >> result = self.cachesig = SCons.Util.MD5collect(sigs)
> >> File "/usr/lib/scons/SCons/Util.py", line 1550, in MD5collect
> >> return MD5signature(', '.join(signatures))
> >> TypeError: sequence item 0: expected str instance, bytes found
> >> scons: building terminated because of errors.
> >> _______________________________________________
> >> 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/20171122/df64bfca/attachment-0001.html>
More information about the Scons-users
mailing list