hugo: helpers/content.go: call rst2html directly to render content #47600

shreyanshk · 2018-10-01T14:44:47Z

Motivation for this change

Hugo launches rst2html by calling Python interpreter and passing it the path of rst2html file.
This doesn't work on NixOS because the real rst2html is actually wrapped behind a bash script which is responsible for correctly setting the env and then exec-ing the real rst2html.

The problem is that rst2html is actually a bash script and not a Python script. Hence, Python fails to launch it with SyntaxError.

This patch ensures that rst2html is called directly so the shebang work correctly.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Determined the impact on package closure size (by running nix path-info -S before and after)
Fits CONTRIBUTING.md.

Mic92 · 2018-10-01T17:41:56Z

pkgs/applications/misc/hugo/010-call-rst2html-directly.patch

+ }
+
+-func getPythonExecPath() string {
+-	path, err := exec.LookPath("python")


This could be even incorrect if python is not the correct version python version for rst2html. Can you also open an issue upstream? They really only should do this on windows in my opinion and even there this probably also will work if rst2html is associated with the correct extension (i.e. rst2html.py).

Also they check if rst2html is in $PATH and is executable. Is this even true on windows?

After digging in LookPath on windows it appears that .py has to be in PATHEXT to be found by LookPath. This means if it is not already a registered extension it would not be found anyway. If it is registered then prepending python should be not necessary.

I know. I am unable to follow their reasoning as well.
I opened an issue upstream like a month ago but haven't received any response at all.
I think it's maybe because Nix is not on their priority list as they have quite a many other issues open. So, maybe we can apply the patch ourselves.

Of all the platforms where Nix works, except Windows, all natively support shebangs.
And, on Windows, Nix works under WSL (experimental) which supports shebangs. So, I believe, we can directly call rst2html for our use case without any issues.

gohugoio/hugo#5273

Do you want to wait a few days/hours for them to merge or do you want to go ahead with the current version?

Ideally, it should be merged. Hugo's next release is at least a month away.

The idea was to use fetchpatch instead when it is merged in hugo master.

Oh! let's wait for upstream to merge it.

The community looks active hopefully this will not take long.

Mic92 · 2018-10-12T09:46:58Z

pkgs/applications/misc/hugo/default.nix

@@ -12,6 +12,9 @@ buildGoPackage rec {
    rev    = "v${version}";
    sha256 = "0n27vyg66jfx4lwswsmdlybly8c9gy5rk7yhy7wzs3rwzlqv1jzj";
  };
+  patches = [
+    ./010-call-rst2html-directly.patch


Ok. Now this can fetched from the upstream repository.

Initially, rst2html was called via the python interpreter which would fail if the script was wrapped in a launcher as on NixOS.

Mic92 · 2018-10-12T17:13:37Z

pkgs/applications/misc/hugo/rst.nix

+{ stdenv, hugo, python3, which }:
+
+stdenv.mkDerivation rec {
+  name = "hugoWithRST";


Do we really need this as a separate package? All it does is adding pygments. Users who need hugo + docutils + pygments can just install both or write a shell.nix for their project.

I tried. Installing both packages doesn't work. Hugo is able to find docutils but not pygments.
What I did was:

nix-env -iA nixpkgs.hugo nix-env -iA nixpkgs.python36Packages.docutils nix-env -iA nixpkgs.python36Packages.pygments

But, that resulted in hugo reporting that it couldn't find pygments.

It looks like docutils needs pygments and it is unable to find it even if both are installed (but separately) because pygments is not on docutils's PATH (or PYTHONPATH).

Additionally, sure shell.nix works but then shell customization are lost. So, I added it.

I usually use direnv to preserve my shell: https://github.com/direnv/direnv/wiki/Nix

It could be made a build option of hugo or a wrapper like weechat, but I would not clutter all-packages.nix with all possible dependency combinations.

This approach is certainly simpler. I'll remove the extra package. Thank you.

@Mic92 , I've updated the PR. Thanks.

shreyanshk · 2018-10-18T11:20:43Z

@Mic92 ,
When would this patch be added into the nixos-18.09 channel? I see the channel was updated some time ago but the patch is still not present into that branch.
Thanks.

Mic92 · 2018-10-18T11:55:14Z

I have not backported the commit back then to 18.09. This is now the case: f8847fc

GrahamcOfBorg added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Oct 1, 2018

Mic92 reviewed Oct 1, 2018

View reviewed changes

Mic92 reviewed Oct 12, 2018

View reviewed changes

hugo: apply upstream patch to call rst2html directly on Nix

d4f88f1

Initially, rst2html was called via the python interpreter which would fail if the script was wrapped in a launcher as on NixOS.

Mic92 reviewed Oct 12, 2018

View reviewed changes

GrahamcOfBorg added the 8.has: package (new) label Oct 12, 2018

Mic92 merged commit 2993047 into NixOS:master Oct 13, 2018

Janik-Haag added the 12. first-time contribution label Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hugo: helpers/content.go: call rst2html directly to render content #47600

hugo: helpers/content.go: call rst2html directly to render content #47600

shreyanshk commented Oct 1, 2018 •

edited

Mic92 Oct 1, 2018

Mic92 Oct 1, 2018

Mic92 Oct 1, 2018

shreyanshk Oct 2, 2018

Mic92 Oct 2, 2018

Mic92 Oct 3, 2018

shreyanshk Oct 4, 2018

Mic92 Oct 4, 2018 •

edited

shreyanshk Oct 5, 2018

Mic92 Oct 5, 2018 •

edited

Mic92 Oct 12, 2018

Mic92 Oct 12, 2018 •

edited

shreyanshk Oct 12, 2018

Mic92 Oct 13, 2018

Mic92 Oct 13, 2018

shreyanshk Oct 13, 2018

shreyanshk Oct 13, 2018

shreyanshk commented Oct 18, 2018

Mic92 commented Oct 18, 2018

hugo: helpers/content.go: call rst2html directly to render content #47600

hugo: helpers/content.go: call rst2html directly to render content #47600

Conversation

shreyanshk commented Oct 1, 2018 • edited

Motivation for this change

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 Oct 4, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 Oct 5, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 Oct 12, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shreyanshk commented Oct 18, 2018

Mic92 commented Oct 18, 2018

shreyanshk commented Oct 1, 2018 •

edited

Mic92 Oct 4, 2018 •

edited

Mic92 Oct 5, 2018 •

edited

Mic92 Oct 12, 2018 •

edited