slapos-sr-testing: do not end paths with /

This may lead to weird like ...//... or worse: here, the [gcc] part of
the tested SR will do "os.path.dirname('/usr/bin/')" (when using system
gcc), causing ${gcc:prefix} to be invalid ('/usr/bin' instead of '/usr')
and most sharable parts will rebuild (different [gcc] signature).

software/slapos-sr-testing/buildout.hash.cfg

@@ -15,4 +15,4 @@
 
 [template]
 filename = instance.cfg
-md5sum = 9639d0c0e161c094454808fb95fc9781
+md5sum = 5ba23849f0b9bcfbe2534314ce7f48e8

software/slapos-sr-testing/instance.cfg

@@ -36,7 +36,7 @@ nxdtest-working-dir = ${:var}/nxdtest
 recipe = slapos.recipe.template:jinja2
 rendered = ${directory:etc}/${:_buildout_section_name_}
 template = inline:
-  export PATH={{ buildout['bin-directory'] }}:{{ curl_location }}/bin/:{{ faketime_location }}/bin/:{{ openssl_location }}/bin/:/usr/bin/:/bin
+  export PATH={{ buildout['bin-directory'] }}:{{ curl_location }}/bin:{{ faketime_location }}/bin:{{ openssl_location }}/bin:/usr/bin:/bin
   export SLAPOS_TEST_IPV4=${slap-configuration:ipv4-random}
   export SLAPOS_TEST_IPV6=${slap-configuration:ipv6-random}
   export SLAPOS_TEST_WORKING_DIR=${directory:test-working-dir}

[Julien Muchembled]

And the time to run the re6stnet line in slapos-sr-testing now takes 579s instead of 1900s.

/cc @nexedi

[Vincent Pelletier]

[gcc] part of the tested SR will do "os.path.dirname('/usr/bin/')"

This level of sensibility to trailing separators looks like a bug to me. Maybe that code should become:

>>> os.path.dirname(os.path.normpath('/usr/bin'))
'/usr'
>>> os.path.dirname(os.path.normpath('/usr/bin/'))
'/usr'

[Julien Muchembled]

No, I insist on the fact that we must stop approximate/good-enough programming, whereas we can just start with canonical forms of everything. Sanitizing input is difficult so let's limit that to what humans submit. For the rest, I prefer strict API.

Another common ugly thing is code listing all possible representations of boolean values, I mean 0/false/no/1/true/yes, multiplied by many ways of having capital letters and also string variants ("False" / False), all this being copied&pasted everywhere but the list is never complete and always differs so it's a huge mess and when a value does not match, it's random rather than failing. IOW, the perfect recipe for a disaster.

[Vincent Pelletier]

About the path: I do not understand your argument: what is being fixed here is the perfect illustration of why code should sanitise human input, because humans cause issues which go unnoticed for at least months: it just worked, for some definition of "worked". Or are you arguing for something like assert value == os.path.normpath(value) so that incorrect human input does fail ? Then why not just make it work instead of failing ? It is not like the intent behind either form of the value is ambiguous.

About booleans: You brought this subject up from nowhere.

[Julien Muchembled]

what is being fixed here is the perfect illustration of why code should sanitise human input, because humans cause issues which go unnoticed for at least months

No, because I don't consider that software/slapos-sr-testing/instance.cfg contains human input. Here, it's for me simply a bug. But I admit that there can be other cases where it's actually human input (the user customized his environment and added a trailing / in $PATH).

Or are you arguing for something like assert value == os.path.normpath(value) so that incorrect human input does fail ?

Maybe. Not sure. Well, I'd like to keep the existing [gcc] code as it is, as simple as possible. I'd be surprise if there wasn't identical bugs elsewere: are you ready to find them ? I see paths with double-/ quite often in SlapOS. I find much simpler to avoid fancies (which simply consists in following a simple rule) rather than adding os.path.normpath absolutely everywhere, just in case.

I see that Py3's shutil.which (which we'll use at some point) does use os.path.normpath so I guess I have no choice.

Then why not just make it work instead of failing ? It is not like the intent behind either form of the value is ambiguous.

But that's exactly what the parsing of booleans tries to do and fails badly. I gave this example because I think it describes the general issue of trying to accept anything.