Bug 208970 – [GTK] Use ${PYTHON_EXECUTABLE} to run generate-gtkdoc

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Michael Catanzaro]

Fedora and Ubuntu are both patching generate-gtkdoc to use #!/usr/bin/python3 rather than #!/usr/bin/env python.

#!/usr/bin/env and /usr/bin/python are both banned in Fedora packages. I guess Ubuntu probably has similar rules.

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.

[Michael Catanzaro]

Created attachment 393330 [details]
Patch

[Daniel Bates]

https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_multiple_python_runtimes

[Daniel Bates]

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.

[Michael Catanzaro]

Created attachment 393372 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 393372 [details]
Patch for landing

Clearing flags on attachment: 393372

Committed r258328: <https://trac.webkit.org/changeset/258328>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ting-Wei Lan]

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.

[Michael Catanzaro]

(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.

[Michael Catanzaro]

Created attachment 394167 [details]
Patch

[EWS]

Committed r258808: <https://trac.webkit.org/changeset/258808>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394167 [details].