Closed Bug 1541409 Opened 6 months ago Closed 5 months ago

hooks_clang_format fails on multiple files

Categories

(Firefox Build System :: Lint and Formatting, defect)

defect
Not set

Tracking

(firefox68 fixed)

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: janerik, Assigned: Sylvestre)

References

(Regression)

Details

Attachments

(1 file)

I recently added hooks_clang_format.py to run as the pre-commit hook on my git checkout of m-c.
However, it seems to have problems when multiple files are stages for the commit:

I get this output:

fatal: pathspec 'browser/base/content/test/about/browser_aboutCertError_telemetry.js browser/base/content/test/siteIdentity/browser_identityPopup_clearSiteData.js [...many more files]' did not match any files

I suspect it is because the files are concatenated into a single space-separated string and then passed to git as one argument.

git expects each file as its own argument though. Guess we should change add_remove_files to take a list instead?

Interesting. Thanks for reporting that.

Btw, I should probably filter out js files in the hook itself.

Regressed by: 1525725

ah yes, guess clang-format wouldn't need to touch these ever, so making it do less work is a sensible thing to do.

Duplicate of this bug: 1538990

For now, I am fixing the core issue. I will create a follow up to only format supported files.
To test my change:

sed -i -e "s|#include |#include    |g" dom/presentation/AvailabilityCollection.cpp
sed -i -e "s|#include |#include    |g" dom/presentation/Presentation.cpp
git add dom/presentation/AvailabilityCollection.cpp dom/presentation/Presentation.cpp
git commit -m "foo"
Regressed by: 1514770
No longer regressed by: 1525725
Duplicate of this bug: 1542799
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/400fd14b1c83
git clang-format hook: add files one by one to avoid an error r=sheehan
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee: nobody → sledru
You need to log in before you can comment on or make changes to this bug.