xmonad: allow man page installation without "enableSeparateDocOutput"

[dudebout]

e9e9669 required enableSeparateDataOutput and enableSeparateDocOutput to both be true to work. Furthermore, it relied on the strange fact that xmonad puts the man page in data-dir (https://hackage.haskell.org/package/xmonad-0.13/xmonad.cabal). Given that this is not used from the source code, this is likely a bug in XMonad and I will follow up on this.

We can instead pick the man page directly from the source directory ..

Using $out will still work with split outputs. multiple-outputs.sh:145 will move the man page from $out to ${outputMan}

[peti]

If we merge this patch now as-is, won't the man page end up being installed twice?

[dudebout]

The man page will indeed be installed twice. However, one of the installations is through data-dir which is supposed to indicate a runtime dependency. Taking the file away from data-dir does not seem like something we should do.

As a side note, the automatic moving of outputs performed by multiple-outputs.sh is problematic with enableSeparateDataOutput. Suppose xmonad wanted to display that man page and used getDataDir or getDataFilePath to locate the file. If the file had been in the tarball under share/man instead of man, multiple-outputs.sh would have automatically moved it to the "man" or "doc" output if present.

[peti]

Taking the file away from data-dir does not seem like something we should do.

Why not? We have done that for the last couple of years and there was no problem. My guess is that xmonad installs this via data-files simply because its authors don't know how to install that man page properly via Cabal. There is no indication this file is ever actually loaded a run-time by xmonad.

[dudebout]

I think this discussion about double install of the man page is taking us away from the main point of this patch. Let me postpone my thoughts on the matter for the second part of my reply. The main problem is that the postInstall script uses the $data variable which will not be present if the user overrides enableSeparateDataOutput to false. Therefore, we need to pick the man page from .. A similar comment applies regarding the $doc variable, which is why it is important to place the man page in $out and rely on multiple-output.sh to move it to ${outputMan}.

I agree that in this case, taking from data-files does not break anything. It just feels conceptually wrong, given the semantics of data-files to remove files from it. It seems like the right thing to do is to reach out to upstream to let them know that it is likely a bug: the three files (xmonad.1, xmonad.1.html, xmonad.hs) in data-files are not used from the code, and putting a file in data-dir does not actually ensure that it gets installed in the "right" place (man page in share/man, HTML and example in share/doc).

If we absolutely want to avoid duplication, we need to find a way that works whether or not $data is defined.

By the way, the current postInstall script creates a data output only containing an empty man directory.

[peti]

The main problem is that the postInstall script uses the $data variable, which will not be present if the user overrides enableSeparateDataOutput to false.

I don't perceive that as a problem at all. The generated expression is valid. No issue there. Now, if someone overrides enableSeparateDataOutput -- which sounds like a very theoretical problem, because I can't think of any reason why someone would do that --, then it's their responsibility to make sure the modified expression still works, i.e. they have to fix the postInstall hook to appropriately. I agree that it would be nice if the postInstall hook worked with or without enableSeparateDataOutput, but I don't perceive that as a very important problem.

The right thing to do is to reach out to upstream to let them know that [installing] the three files (xmonad.1, xmonad.1.html, xmonad.hs) in data-files [is a bug].

Absolutely. I completely agree, and it would be great if someone would do that.

If we absolutely want to avoid duplication, we need to find a way that works whether or not $data is defined.

Yes, that would be preferable.

[dudebout]

I have reported the issue upstream: xmonad/xmonad#127

I have also opened an issue in the nixpkgs repo which shows that the issue is not purely theoretical; I was hit by it while trying to fix a breakage in xmonad-with-packages.

If we add the following two lines at the end of the patched post-install script, we will get the same output as the pre-patch version:

if [[ -z $data ]]; then data=$out; fi
rm "$data/"**"/man/"*"

What do you think?

[peti]

I think that xmonad-with-packages should be fixed to deal with multiple outputs. Disabling that feature to make broken code work again is not the correct way to deal with this issue.

[dudebout]

But my patch (with the 2 lines mentioned in my previous reply) does not disable anything.

[peti]

Your patch aims to make the package work in the presence of disabled doc outputs. I don't think the separate output should be disabled, and I haven't heard a good reason yet why anyone might want to. Therefore, I don't think this patch is very desirable to have.

[dudebout]

I see the confusion.

See the last line in my initial comment. I am changing the postInstall script, but not the actual default behavior. There is already code in multiple-output.sh which will take the man pages and put them in the doc output if you have this output enabled. There is no need for you to manually put it in the $doc directory. Putting it in the $doc directory only makes the code not work for users wanting to set the public flag enableSeparateDocOutput to false, without changing anything for the default behavior.

[peti]

Actually, the confusion comes from the fact that you see the name "enableSeparateDocOutput" and think, "aha, this looks like something I can pass false to to disable it". But this is wrong. You can generally not disable separate outputs for data or documentation without breaking the build. We don't want people to disable separate outputs and we've made no effort whatsoever to support that. Therefore, I don't want to merge your patch, because it tries to create support for something we don't want to support.

[dudebout]

OK. FYI, the xmonad package build will break when a user disables haddock since `enableSeparateDocOutput` has a default value of `doHaddock`.

…

On Aug 7, 2017 3:31 AM, "Peter Simons" ***@***.***> wrote: Actually, the confusion comes from the fact that you see the name "enableSeparateDocOutput" and think, "aha, this looks like something I can pass false to to disable it". But this is wrong. You can generally not disable separate outputs for data or documentation without breaking the build. We don't want people to disable separate outputs and we've made no effort whatsoever to support that. Therefore, I don't want to merge your patch, because it tries to create support for something we don't want to support. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#290 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAXVmu-bz_mlexqJCpRt8ieTCQ-IXwt8ks5sVr1ZgaJpZM4Otxrg> .

[dudebout]

Is there any documentation or discussion about the intent of enableSeparateDocOutput and co.? The only thing I found was the commit where it was enabled: NixOS/nixpkgs@be63b19

[peti]

Some record of the discussion that took place is in NixOS/nixpkgs#27196. Other conversations happened in the IRC channel and are probably not easy to find.

[dudebout]

Thank you