Mysterious presubmit --all bot failures

[Bruce Dawson]

I'm probably missing something obvious, but I can't figure this out. The Linux presubmit --all bot just started failing, with the first failure being here:

https://ci.chromium.org/ui/p/chromium/builders/ci/linux-presubmit/1742/overview

The failure is in tools/json_schema_compiler/PRESUBMIT.py and the blamelist shows that crrev.com/c/2215902 modified that file so, case closed, right?

Except that I can't see anything wrong with the change, and when I test it locally, on Linux or Windows (after pulling to the latest code) it succeeds.

This is my test command and I've confirmed that it runs the code that fails on the bot. Any ideas?

    git cl presubmit --force --files tools/json_schema_compiler/PRESUBMIT.py

It seems dangerous to have the bot finding failures that don't happen on developer machines so I'd really like to understand this.

--

Bruce Dawson, he/him

[Andrew Grieve]

Looks like json_schema_compiler/code.py is what it's wanting to import, but instead it's ending up importing /b/s/w/ir/cipd_bin_packages/3.8/lib/python3.8/code.py.

I suspect that means this line is not working as desired:

sys.path.insert(0, input_api.PresubmitLocalPath())

I suspect using AbsoluteLocalPath() might fix it.

 

--
You received this message because you are subscribed to the Google Groups "python" group.
To unsubscribe from this group and stop receiving emails from it, send an email to python+un...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/python/CAE5mQiOwhZ0xNQk27GKtVtEFJNHZGW66%2BJtx-UDMOFuVrVQndw%40mail.gmail.com.

[Mike Frysinger]

there is no AbsoluteLocalPath() afaict.  there is AbsoluteLocalPaths(), but that is the list of affected files, not the path to the script itself (which is what we need).

when i run it locally, PresubmitLocalPath shows an absolute path ...

wonder if it'll be easier to rename the custom code.py module so it doesn't shadow system Python modules.

-mike

[Andrew Grieve]

I think it's defined here:

https://source.chromium.org/chromium/chromium/src/+/main:third_party/depot_tools/presubmit_support.py;drc=9f38b63b4e343d9600d2dc173ab4f59d14ab6713;l=997

I agree renaming the file seems like a good solution though. It's generally not good practice to collide with system module names (although can be difficult since new system modules get added with common names :P)

[Mike Frysinger]

that AbsoluteLocalPath is a method on the AffectedFile class, and affected files are the things being changed.  this code needs the path to the presubmit file itself that's being triggered, not the files it's processing.

i'll see about renaming the module.

-mike

[Andrew Grieve]

Ahh, that makes sense.

[Bruce Dawson]

The comments around PresubmitLocalPath() could use improving. The function returns the directory containing the currently active PRESUBMIT.py files, but it just says "the local path", which can easily be interpreted as the file path, not directory path. I might tweak the comments, but it should be returning the correct/expected directory.

My local testing shows that it is correctly injecting the correct absolute path into the front of sys.path. There must be something odd going on with the bot configuration. This page says that the search order is built-in python modules, then sys.path entries, but that doesn't explain the differences.

I agree with renaming the module, even if we don't fully understand the issue. Thanks Mike.

--

Bruce Dawson, he/him

[Bruce Dawson]

Renaming the module (crrev.com/c/4092431) worked. It's still not clear to me why the presubmit passed locally but failed on the bots, but I'm not sure that understanding that is critical.

Thanks for the help.

--

Bruce Dawson, he/him