how to fix git cl format warnings

235 views
Skip to first unread message

Xing

unread,
Oct 4, 2016, 9:13:22 AM10/4/16
to Chromium-dev
Hi, I am trying to rebase one CL. But unluckily I first use git pull --rebase and when conflicts, I tried git merge --abort.
And after this, I successfully git cl upload.  But when dry run,  it outputs warning for chromium_presubmit:
"
** Presubmit Warnings **
The WebKit directory requires source formatting. Please run git cl format third_party/WebKit
"

Then I trid git cl format locally like this, for file I didn't change, nothing output. For files changed by me, it outputs permission denied.
Also I tried git reset 'HEAD@{41}', problem still not resolved.

Any hints for this?

The detail output for git cl format:
src$ git cl format third_party/WebKit/Source/core/html/HTMLSummaryElement.cpp
Traceback (most recent call last):
  File "/g1/google/chromiumlinux_0815/src/buildtools/clang_format/script/clang-format-diff.py", line 121, in <module>
    main()
  File "/g1/google/chromiumlinux_0815/src/buildtools/clang_format/script/clang-format-diff.py", line 104, in main
    stderr=None, stdin=subprocess.PIPE)
  File "/usr/lib/python2.7/subprocess.py", line 710, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1327, in _execute_child
    raise child_exception
OSError: [Errno 13] Permission denied
Command "/usr/bin/python /g1/google/chromiumlinux_0815/src/buildtools/clang_format/script/clang-format-diff.py -p0 -i" failed.


src$ git cl format third_party/WebKit/Source/core/html/HTMLDetailsElement.cpp
Nothing outputs.

Nico Weber

unread,
Oct 4, 2016, 9:45:35 AM10/4/16
to xin...@intel.com, Chromium-dev
What OS are you on? What's the output of `ls -l buildtools/{mac,linux64}/`? Did you run `gclient sync`?

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

Xing

unread,
Oct 4, 2016, 10:26:14 AM10/4/16
to Chromium-dev, xin...@intel.com
Did run git rebase-update/gclient sync.  

src$ ls -l buildtools/linux64/
total 3236
-rw-rw-r-- 1 abc abc 1828080 10月  4 08:58 clang-format
-rw-rw-r-- 1 abc abc      40  9月 27 11:54 clang-format.sha1
-rwxrw-r-- 1 abc abc 1473232 10月  4 16:32 gn
-rw-rw-r-- 1 abc abc      41 10月  4 16:31 gn.sha1



在 2016年10月4日星期二 UTC+8下午9:45:35,Nico Weber写道:

Xing

unread,
Oct 4, 2016, 10:28:15 AM10/4/16
to Chromium-dev, xin...@intel.com
OS is ubuntu 1404

在 2016年10月4日星期二 UTC+8下午10:26:14,Xing写道:

Primiano Tucci

unread,
Oct 4, 2016, 10:31:53 AM10/4/16
to xin...@intel.com, Chromium-dev
 clang-format should have exec permissions. You can fix it by manually chmod 755 buildtools/linux64/clang-format but you are not supposed to do that manually.
What filesystem is  /g1/google/chromiumlinux_0815 on? It smells like either that is a vfat filesystem (or anything which doesn't suppose exec permissions) or it is mounted with noexec.
Chrome expects the filesystem you use to check out the code to allow execution.

Nico Weber

unread,
Oct 4, 2016, 10:36:04 AM10/4/16
to xin...@intel.com, Chromium-dev
Strange, clang-format needs x set. `chmod +x buildtools/linux64/clang-format` will fix, but that should Just Work. (gn does have the right bit set.)

Does `rm buildtools/linux64/clang-format && gclient runhooks && ls buildtools/linux64` change anything?

The file does have x set on storage:

$ ~/gsutil/gsutil ls -L gs://chromium-clang-format/$(cat buildtools/linux64/clang-format.sha1) | grep executable
executable: 1

Xu, Xing

unread,
Oct 4, 2016, 10:44:55 AM10/4/16
to Primiano Tucci, Chromium-dev

File system is ext4.

/dev/sdb1 on /g1 type ext4 (rw,errors=remount-ro)

 

 

But this works on most folder, such cc/net/ui, but only failed at  git cl format third_party/WebKit/Source/core/html/HTMLSummaryElement.cpp

 

 

Regards,

Xing

Xing

unread,
Oct 4, 2016, 11:02:17 AM10/4/16
to Chromium-dev, prim...@chromium.org
Thanks all your guys. After apply chmod +x buildtools/linux64/clang-format, git cl format  third_party/WebKit/Source/core/html/HTMLSummaryElement.cpp.

And I understand that git cl format only check modified files. That's why git cl cc/net/ui get passed but only this summary file fail.


But, git cl upload will do Presubmit checks, why does local get passed but failed during CQ dry run(chromium_presubmit)?




在 2016年10月4日星期二 UTC+8下午10:44:55,Xing写道:

Christian Biesinger

unread,
Oct 4, 2016, 11:06:15 AM10/4/16
to xin...@intel.com, Chromium-dev, Primiano Tucci
It may have failed for the same reason, maybe? It couldn't execute
clang-format, so it couldn't check the formatting, but it continued
on. That's just a guess though.

-Christian

Primiano Tucci

unread,
Oct 4, 2016, 11:10:49 AM10/4/16
to Christian Biesinger, xin...@intel.com, Chromium-dev
> But, git cl upload will do Presubmit checks, why does local get passed but failed during CQ dry run(chromium_presubmit)?

There are two different types of presubmit checks: pre-upload (the one that run locally) and pre-push (The one that the CQ runs). The more expensive ones are done only on pre-push, as they would make the git cl upload experience too miserable. Perhaps that is one of those.

I believe (but not 100% sure) that git cl presubmit will do the expensive pre-push hooks, while by default git cl upload does only "git cl presubmit -u" which are the lighter ones.

Christian Biesinger

unread,
Oct 4, 2016, 11:28:04 AM10/4/16
to Primiano Tucci, xin...@intel.com, Chromium-dev
I can tell you from experience that git cl upload does run the clang
format heck.

Christian

Tomasz Sniatowski

unread,
Oct 5, 2016, 3:33:14 AM10/5/16
to Chromium-dev, xin...@intel.com
Random missing +x on files downloaded from google storage could be caused by https://bugs.chromium.org/p/chromium/issues/detail?id=645396 -- an abort at just the wrong time can leave a file downloaded fully, but without the executable bit set, and this situation won't correct itself automatically.

Paweł Hajdan, Jr.

unread,
Oct 5, 2016, 4:02:07 AM10/5/16
to tsnia...@opera.com, infr...@chromium.org, Ryan Tseng, Chromium-dev, xin...@intel.com
+infra-dev,hinoka

Yeah, the suggestion on the bug (perhaps download_from_google_storage should download to a temporary filename, chmod, and then mv to the final location) sounds good to me.

FWIW, handling downloaded archives would still be somewhat tricky if the process gets killed in the middle of extracting the archive. :-/

Paweł

Tomasz Śniatowski

unread,
Oct 5, 2016, 4:13:50 AM10/5/16
to Paweł Hajdan, Jr., infr...@chromium.org, Ryan Tseng, Chromium-dev, xin...@intel.com
Archives can be handled in a similar fashion, for example using the actual archive file as a guard to mean "unpacking may have not finished, redo and then mv/rm the archive".

--
Tomasz Śniatowski

Primiano Tucci

unread,
Oct 5, 2016, 11:35:46 AM10/5/16
to Tomasz Śniatowski, Paweł Hajdan, Jr., infr...@chromium.org, Ryan Tseng, Chromium-dev, xin...@intel.com
Like it the case of a temp file it should:
- uncompress into a X.tmp folder sitting next to the original one (X)
- rename X -> X.delete
- rename X.tmp -> X
- rm -rf X.delete

which reduces sensibly the race window, and in any case never leaves around a stale folder (worst case you get no folder at all) 

--
You received this message because you are subscribed to the Google Groups "infra-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+...@chromium.org.
To post to this group, send email to infr...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-dev/CAMJuQOzuR3FYkkeQoBu%2Bmn79UFqDu9z1xdpdMoc-5m2BO7uZdg%40mail.gmail.com.
Reply all
Reply to author
Forward
0 new messages