| Summary: | [GTK] Use ${PYTHON_EXECUTABLE} to run generate-gtkdoc | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> | ||||||||
| Component: | WebKitGTK | Assignee: | Michael Catanzaro <mcatanzaro> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | annulen, bugs-noreply, commit-queue, ews-watchlist, gyuyoung.kim, lantw44, mcatanzaro, ryuan.choi, sergio | ||||||||
| Priority: | P2 | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Linux | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Michael Catanzaro
2020-03-11 19:38:05 PDT
Created attachment 393330 [details]
Patch
Comment on attachment 393330 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393330&action=review This change looks good. > Tools/ChangeLog:10 > + Fedora and Ubuntu are both patching generate-gtkdoc to use #!/usr/bin/python3 rather than This is ok as-is. No change is needed. The optimal ChangeLog would move all the text in this line and later ABOVE the file name so as to be the description of the change because: 1. There is only one file changed in this patch so there's no need for per file comments. 2. Per file/function comments should be concise, no longer than a single paragraph. They are meant to provide more fine grain descriptions of the charges in the patch. > Tools/ChangeLog:13 > + #!/usr/bin/env and /usr/bin/python are both banned in Fedora packages. I guess Ubuntu This is ok as-is. No change is needed. A slightly better description would reference the official documentation by URL, along with date accessed (if the URL doesn't look permanent - encode a date or version) and optionally include an excerpt from it if the URL does not look permanent. In other words, a slightly better description provides irrefutable proof why the change was made in a way that a third party in the future can verify the claim. Created attachment 393372 [details]
Patch for landing
Comment on attachment 393372 [details] Patch for landing Clearing flags on attachment: 393372 Committed r258328: <https://trac.webkit.org/changeset/258328> All reviewed patches have been landed. Closing bug. This breaks build-webkit script on FreeBSD. I typed "./Tools/Scripts/build-webkit --gtk --cmakeargs='-DENABLE_GTKDOC=ON -DENABLE_MINIBROWSER=ON -DENABLE_TOOLS=ON -DUSE_SYSTEMD=OFF' --no-experimental-features" and it failed at generate-gtkdoc with "No such file or directory" because there is no /usr/bin/python3. (In reply to Michael Catanzaro from comment #0) > Fedora and Ubuntu are both patching generate-gtkdoc to use > #!/usr/bin/python3 rather than #!/usr/bin/env python. I think we can just use "#!/usr/bin/env python3" to avoid the dependency on the version-less python executable. > #!/usr/bin/env and /usr/bin/python are both banned in Fedora packages. I > guess Ubuntu probably has similar rules. "#!/usr/bin/env" is only banned in packaged files. For the source code, I think it should be acceptable to rely on PATH to find executables used during the build. > Note this is the only script that attempts to use /usr/bin/python in tarball > builds. Everything else that does so is probably only used by webkit-build. (In reply to Ting-Wei Lan from comment #7) > "#!/usr/bin/env" is only banned in packaged files. For the source code, I > think it should be acceptable to rely on PATH to find executables used > during the build. You're right. Because the script is not installed, we are indeed allowed to use #!/usr/bin/env. Created attachment 394167 [details]
Patch
Committed r258808: <https://trac.webkit.org/changeset/258808> All reviewed patches have been landed. Closing bug and clearing flags on attachment 394167 [details]. |