Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

This enables testing with stackage resolvers #268

Closed
wants to merge 1 commit into from

Conversation

psibi
Copy link
Member

@psibi psibi commented Jan 20, 2021

No description provided.

@psibi
Copy link
Member Author

psibi commented Jan 20, 2021

@peti Can I know what objections you have for this MR ?

The man page generation will be resolved by this MR: #260


flags:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't drop these flags from the stack.yaml file. It is important that this flag is enabled because without it continuous builds will not include the genmanpage executable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peti This is going to be addressed by this MR: #260

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#260 sucks and I don't want that change in our code. There are various reasons to reject that change, but the most important one is that it drops knowledge from our Cabal file about an explicit dependency on pandoc and turns that into a hidden dependency on pandoc being $PATH for some reason. Since our build tools don't know about that requirement, it means that builds will fail. In it's current form, the change is certainly going to break the Nix and NixOS builds, for example. It might also break the openSUSE Linux build, but I'm haven't actually checked the latest spec file there so I'm not entirely sure about that. Anyhow, implicit dependency suck and introducing them into our builds is the wrong direction to go.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already implicitly depend on libx11-dev etc. so that argument is not as good as you think it is.
Additionally, it was you who said that generatemanpage is meant to be run by devs only, so the implicit dependency might not be a problem for distros at all, with the possible exception of Debian which may be needing to rebuild the manpage for policy reasons.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, breaking the package build isn't the end of the world. For nix, it's probably trivially resolved by adding pandoc to the list of nix packages in stack.yaml, and distros like Debian will be more than happy to switch from patching xmonad to having one additional build-dep.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already implicitly depend on libx11-dev etc.

We don't depend on libx11-dev at all. xmonad depends on the Haskell library X11, which in turn depends on a couple of system libraries that are explicitly declared as dependencies in it's Cabal file at https://github.com/xmonad/X11/blob/master/X11.cabal#L74. The change you proposed drops a direct dependency of genmanpage from our Cabal file.

Additionally, it was you who said that generatemanpage is meant to be run by devs only, so the implicit dependency might not be a problem for distros at all, with the possible exception of Debian which may be needing to rebuild the manpage for policy reasons.

You realize that you actually don't know that, right? You keep making this claim that Debian "needs" to rebuild the man page, but as a matter fact you have no freaking clue whether that's actually true or not.

Copy link
Contributor

@peti peti Jan 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, breaking the package build isn't the end of the world.

Oh? It appears that you worry about our support for Nix far less than you worry about our support for Debian. Interesting.

For nix, it's probably trivially resolved by adding pandoc to the list of nix packages in stack.yaml.

No, that won't help a bit. Trust me on this. I know a thing or two about how xmonad is packaged in Nix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@psibi, why did you mark this conversation as resolved even though it clearly isn't?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought my second comment actually resolved it, but it supposedly didn't. :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't depend on libx11-dev at all. xmonad depends on the Haskell library X11, which in turn depends on a couple of system libraries that are explicitly declared as dependencies in it's Cabal file at https://github.com/xmonad/X11/blob/master/X11.cabal#L74. The change you proposed drops a direct dependency of genmanpage from our Cabal file.

Oh, mea culpa. Unfortunately in practice the explicit extra-libraries deps are mostly meaningless, as their semantics is undefined (what does X11 mean? why not Xlib, or xlib, or libx11-dev, or xlib-devel, or …?) and therefore distros don't use it. Maybe nix does, I don't know, but the other don't.

So, um, maybe adding pandoc to extra-libraries would be an option? :-)

You realize that you actually don't know that, right?

Yes, indeed I do. I'm not the one rushing any merges (hint: you are). I'm waiting to get some replies on the mailing list thread.

Oh? It appears that you worry about our support for Nix far less than you worry about our support for Debian. Interesting.

For fuck's sake, Peter, stop accusing me of your own wrongdoings!

I care about both, and I'm more than willing to look for an acceptable compromise, which is why I solicited feedback from the Debian packagers and asked for PR feedback before merging the PR. You, on the other hand, quite explicitly said that Debian's packaging is not an issue for you in the slightest, and you keep pushing commits that only consider your own environment.

No, that won't help a bit. Trust me on this. I know a thing or two about how xmonad is packaged in Nix.

Okay, fair enough. So what needs to be done to have pandoc in scope when building xmonad for nix? And is it really that much of an issue? Why does Nix need to build generatemanpage and Debian not? Why do you believe it's okay for Debian to patch xmonad, when at the same time you claim that adding a single dependency to the Nix build is a total no-go?

Are you, by any chance, worrying about our support for Debian far less than you worry about our support for Nix?

@@ -14,7 +14,7 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu-latest]
resolver: [nightly]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't drop Stackage Nightly from the list of supported build environments. It is important that we know our code builds in the Nightly snapshot because that environment is going to turn into the next LTS release, eventually.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point and I'm sorry for not noticing this in #266. I think it could've been addressed in a followup commit and didn't need a revert, though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My plan to include new lts as it's created. But that's a good point and I will update the MR to include that too.

@peti
Copy link
Contributor

peti commented Jan 20, 2021

The man page generation will be resolved by this MR: #260

I disagree with that change for the reasons I've outlined in great detail in the PR.

@psibi
Copy link
Member Author

psibi commented Jan 20, 2021

I disagree with that change for the reasons I've outlined in great detail in the PR.

I see that you haven't replied to this response from @liskin there: #260 (comment)

Also, I agree with the changes there and my intention is to get both of these merged.

@peti
Copy link
Contributor

peti commented Jan 20, 2021

I see that you haven't replied to this response from @liskin there: #260 (comment)

No, I haven't. And I won't because that response doesn't address the points I have made in my original response. I have better things to do than to repeat myself over and over talking to someone who doesn't want to hear what I'm saying.

Also, I agree with the changes there and my intention is to get both of these merged.

Well, good for you.

@liskin
Copy link
Member

liskin commented Jan 20, 2021

I have better things to do than to repeat myself over and over talking to someone who doesn't want to hear what I'm saying.

This feeling is, too, mutual. :-/

Copy link
Contributor

@peti peti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to change my previous review into a neutral one. I'm happy with whatever the outcome of this process is going to be.

@liskin liskin added this to the v0.17 milestone Mar 27, 2021
@liskin liskin added this to In progress in xmonad 0.17 Mar 27, 2021
liskin added a commit to liskin/xmonad that referenced this pull request Mar 31, 2021
This reverts commit 8a8d5f7.

There will be a separate GH Actions workflow for rebuilding the manpage,
and generatemanpage will be dropped from xmonad.cabal (see
xmonad#283) therefore the revert no
longer makes sense and the stack workflow can indeed be reverted back to
a working state so we can continue from there.

For more information, see the following revenge revert storm:

Related: xmonad#283
Related: xmonad#260
Related: xmonad#261
Related: xmonad#265
Related: xmonad#266
Related: xmonad#267
Related: xmonad#268
liskin added a commit to liskin/xmonad that referenced this pull request Mar 31, 2021
pandoc's API changes often enough that distros like Debian were patching
our GenerateManpage.hs to work with their version of pandoc, and it
doesn't build against any Stackage LTS except the recently released
LTS-17. Also, building pandoc from source takes quite some time and
resources.

But for what benefit? We're not using any special pandoc functionality
whatsoever. It's just that it was all in Haskell and the entire build
was orchestrated by cabal, but is all that complexity and resource
consumption worth it? I think not.

(Frankly, this whole thing was just a massive waste of time as the help
text in Config.hs isn't generated at all, so we still need to do this
manually. And then, the default key-bindings in core are unlikely to
change ever again.)

Let's simplify this:

* drop all dependencies except base and just run it through runhaskell

* add a Makefile and GH Actions workflow that invokes this after push

* only ship the results in release tarball, not the scripts which are
  considered our dev infrastructure

Related: xmonad#283
Related: xmonad#260
Related: xmonad#261
Related: xmonad#265
Related: xmonad#266
Related: xmonad#267
Related: xmonad#268
liskin added a commit to liskin/xmonad that referenced this pull request Mar 31, 2021
pandoc's API changes often enough that distros like Debian were patching
our GenerateManpage.hs to work with their version of pandoc, and it
doesn't build against any Stackage LTS except the recently released
LTS-17. Also, building pandoc from source takes quite some time and
resources.

Therefore, separate it into its own cabal project and invoke it in CI to
make sure we're releasing an up-to-date manpage, and don't encourage
downstreams to rebuild it -- it's now just our dev tooling.

(Uncached run of the generatemanpage workflow takes more than half an
hour, which seems incredibly wasteful for what it does. The default
key-bindings in core are unlikely to change ever again, the help text in
Config.hs isn't generated at all, so we still need to do that thing
manually, I'm really not convinced this is worth the trouble...)

Closes: xmonad#283
Related: xmonad#260
Related: xmonad#261
Related: xmonad#265
Related: xmonad#266
Related: xmonad#267
Related: xmonad#268
liskin added a commit to liskin/xmonad that referenced this pull request Mar 31, 2021
pandoc's API changes often enough that distros like Debian were patching
our GenerateManpage.hs to work with their version of pandoc, and it
doesn't build against any Stackage LTS except the recently released
LTS-17. Also, building pandoc from source takes quite some time and
resources.

Therefore, separate it into its own cabal project and invoke it in CI to
make sure we're releasing an up-to-date manpage, and don't encourage
downstreams to rebuild it -- it's now just our dev tooling.

(Uncached run of the generatemanpage workflow takes more than half an
hour, which seems incredibly wasteful for what it does. The default
key-bindings in core are unlikely to change ever again, the help text in
Config.hs isn't generated at all, so we still need to do that thing
manually, I'm really not convinced this is worth the trouble...)

Closes: xmonad#283
Related: xmonad#260
Related: xmonad#261
Related: xmonad#265
Related: xmonad#266
Related: xmonad#267
Related: xmonad#268
liskin added a commit to liskin/xmonad that referenced this pull request Mar 31, 2021
pandoc's API changes often enough that distros like Debian were patching
our GenerateManpage.hs to work with their version of pandoc, and it
doesn't build against any Stackage LTS except the recently released
LTS-17. Also, building pandoc from source takes quite some time and
resources.

Therefore, separate it into its own cabal project and invoke it in CI to
make sure we're releasing an up-to-date manpage, and don't encourage
downstreams to rebuild it -- it's now just our dev tooling.

(Uncached run of the generatemanpage workflow takes more than half an
hour, which seems incredibly wasteful for what it does. The default
key-bindings in core are unlikely to change ever again, the help text in
Config.hs isn't generated at all, so we still need to do that thing
manually, I'm really not convinced this is worth the trouble...)

Closes: xmonad#283
Related: xmonad#260
Related: xmonad#261
Related: xmonad#265
Related: xmonad#266
Related: xmonad#267
Related: xmonad#268
liskin added a commit to liskin/xmonad that referenced this pull request Mar 31, 2021
pandoc's API changes often enough that distros like Debian were patching
our GenerateManpage.hs to work with their version of pandoc, and it
doesn't build against any Stackage LTS except the recently released
LTS-17. Also, building pandoc from source takes quite some time and
resources.

Therefore, separate it into its own cabal project and invoke it in CI to
make sure we're releasing an up-to-date manpage, and don't encourage
downstreams to rebuild it -- it's now just our dev tooling.

(Uncached run of the generatemanpage workflow takes more than half an
hour, which seems incredibly wasteful for what it does, and that's the
reason why I add GenerateManpage.cabal.project.freeze. The default
key-bindings in core are unlikely to change ever again, the help text in
Config.hs isn't generated at all, so we still need to do that thing
manually, I'm really not convinced this is worth the trouble...)

Closes: xmonad#283
Related: xmonad#260
Related: xmonad#261
Related: xmonad#265
Related: xmonad#266
Related: xmonad#267
Related: xmonad#268
liskin added a commit to liskin/xmonad that referenced this pull request Mar 31, 2021
pandoc's API changes often enough that distros like Debian were patching
our GenerateManpage.hs to work with their version of pandoc, and it
doesn't build against any Stackage LTS except the recently released
LTS-17. Also, building pandoc from source takes quite some time and
resources.

But for what benefit? We're not using any special pandoc functionality
whatsoever. It's just that it was all in Haskell and the entire build
was orchestrated by cabal, but is all that complexity and resource
consumption worth it? I think not.

(Frankly, this whole thing was just a massive waste of time as the help
text in Config.hs isn't generated at all, so we still need to do this
manually. And then, the default key-bindings in core are unlikely to
change ever again.)

Let's simplify this:

* drop all dependencies except base and just run it through runhaskell

* add a Makefile and GH Actions workflow that invokes this after push

* only ship the results in release tarball, not the scripts which are
  considered our dev infrastructure

Closes: xmonad#283
Related: xmonad#260
Related: xmonad#261
Related: xmonad#265
Related: xmonad#266
Related: xmonad#267
Related: xmonad#268
@liskin
Copy link
Member

liskin commented Mar 31, 2021

Done in liskin@fb390fa

@liskin liskin closed this Mar 31, 2021
xmonad 0.17 automation moved this from In progress to Done Mar 31, 2021
liskin added a commit to liskin/xmonad that referenced this pull request Apr 1, 2021
pandoc's API changes often enough that distros like Debian were patching
our GenerateManpage.hs to work with their version of pandoc, and it
doesn't build against any Stackage LTS except the recently released
LTS-17. Also, building pandoc from source takes quite some time and
resources.

But for what benefit? We're not using any special pandoc functionality
whatsoever. It's just that it was all in Haskell and the entire build
was orchestrated by cabal, but is all that complexity and resource
consumption worth it? I think not.

(Frankly, this whole thing was just a massive waste of time as the help
text in Config.hs isn't generated at all, so we still need to do this
manually. And then, the default key-bindings in core are unlikely to
change ever again.)

Let's simplify this:

* drop all dependencies except base and just run it through runhaskell

* add a Makefile and GH Actions workflow that invokes this after push

* only ship the results in release tarball, not the scripts which are
  considered our dev infrastructure

Closes: xmonad#283
Related: xmonad#260
Related: xmonad#261
Related: xmonad#265
Related: xmonad#266
Related: xmonad#267
Related: xmonad#268
liskin added a commit to liskin/xmonad that referenced this pull request Apr 1, 2021
pandoc's API changes often enough that distros like Debian were patching
our GenerateManpage.hs to work with their version of pandoc, and it
doesn't build against any Stackage LTS except the recently released
LTS-17. Also, building pandoc from source takes quite some time and
resources.

Therefore, separate it into its own cabal project and invoke it in CI to
make sure we're releasing an up-to-date manpage, and don't encourage
downstreams to rebuild it -- it's now just our dev tooling.

(Uncached run of the generatemanpage workflow takes more than half an
hour, which seems incredibly wasteful for what it does, and that's the
reason why I add GenerateManpage.cabal.project.freeze. The default
key-bindings in core are unlikely to change ever again, the help text in
Config.hs isn't generated at all, so we still need to do that thing
manually, I'm really not convinced this is worth the trouble...)

Closes: xmonad#283
Related: xmonad#260
Related: xmonad#261
Related: xmonad#265
Related: xmonad#266
Related: xmonad#267
Related: xmonad#268
liskin added a commit to liskin/xmonad that referenced this pull request Apr 27, 2021
pandoc's API changes often enough that distros like Debian were patching
our GenerateManpage.hs to work with their version of pandoc, and it
doesn't build against any Stackage LTS except the recently released
LTS-17. Also, building pandoc from source takes quite some time and
resources.

But for what benefit? We're not using any special pandoc functionality
whatsoever. It's just that it was all in Haskell and the entire build
was orchestrated by cabal, but is all that complexity and resource
consumption worth it? I think not.

(Frankly, this whole thing was just a massive waste of time as the help
text in Config.hs isn't generated at all, so we still need to do this
manually. And then, the default key-bindings in core are unlikely to
change ever again.)

Let's simplify this:

* drop all dependencies except base and just run it through runhaskell

* add a Makefile and GH Actions workflow that invokes this after push

* only ship the results in release tarball, not the scripts which are
  considered our dev infrastructure

Closes: xmonad#283
Related: xmonad#260
Related: xmonad#261
Related: xmonad#265
Related: xmonad#266
Related: xmonad#267
Related: xmonad#268
liskin added a commit to liskin/xmonad that referenced this pull request Apr 27, 2021
pandoc's API changes often enough that distros like Debian were patching
our GenerateManpage.hs to work with their version of pandoc, and it
doesn't build against any Stackage LTS except the recently released
LTS-17. Also, building pandoc from source takes quite some time and
resources.

But for what benefit? We're not using any special pandoc functionality
whatsoever. It's just that it was all in Haskell and the entire build
was orchestrated by cabal, but is all that complexity and resource
consumption worth it? I think not.

(Frankly, this whole thing was just a massive waste of time as the help
text in Config.hs isn't generated at all, so we still need to do this
manually. And then, the default key-bindings in core are unlikely to
change ever again.)

Let's simplify this:

* drop all dependencies except base and just run it through runhaskell

* add a Makefile and GH Actions workflow that invokes this after push

* only ship the results in release tarball, not the scripts which are
  considered our dev infrastructure

Closes: xmonad#283
Related: xmonad#260
Related: xmonad#261
Related: xmonad#265
Related: xmonad#266
Related: xmonad#267
Related: xmonad#268
@liskin liskin deleted the reenable-lts-testing branch August 2, 2021 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants