darcs

Patch 1908 WIP: drop custom Setup.hs

Title WIP: drop custom Setup.hs
Superseder Nosy List ganesh
Related Issues
Status in-discussion Assigned To
Milestone

Created on 2019-09-02.22:44:51 by ganesh, last changed 2020-02-14.01:10:33 by bfrk.

Files
File name Status Uploaded Type Edit Remove
pEpkey.asc bfrk, 2020-01-25.17:50:03 application/pgp-keys
pEpkey.asc bfrk, 2020-01-26.16:42:29 application/pgp-keys
pEpkey.asc bfrk, 2020-01-26.16:47:36 application/pgp-keys
pEpkey.asc bfrk, 2020-02-13.23:30:55 application/pgp-keys
patch-preview.txt ganesh, 2019-09-02.22:44:51 text/x-darcs-patch
patch-preview.txt ganesh, 2020-01-25.16:45:38 text/x-darcs-patch
patch-preview.txt bfrk, 2020-02-14.01:10:33 text/x-darcs-patch
unnamed ganesh, 2019-09-02.22:44:51 text/plain
unnamed ganesh, 2020-01-25.16:45:38 text/plain
unnamed bfrk, 2020-02-14.01:10:33 text/plain
wip_-drop-custom-setup_hs.dpatch ganesh, 2019-09-02.22:44:51 application/x-darcs-patch
wip_-drop-custom-setup_hs.dpatch ganesh, 2020-01-25.16:45:38 application/x-darcs-patch
wip_-drop-custom-setup_hs_0.dpatch bfrk, 2020-02-14.01:10:33 application/x-darcs-patch
See mailing list archives for discussion on individual patches.
Messages
msg21326 (view) Author: ganesh Date: 2019-09-02.22:44:51
This is WIP for now, but I wanted to discuss the general principle.

The custom Setup.hs in darcs is a source of endless trouble.
We are or have been doing things that are deprecated, it complicates
dependency solving, and the generated files like release/distributed-context
often cause me trouble with file locking on Windows.

We've simplified a few bits piecemeal over the years but maybe it's time
just take one big leap and drop it entirely.

One big thing that we would definitely lose is darcs --exact-version.
This introspects on the repo at build time to embed the full patch
context + weak hash into the binary. We also wouldn't get the
"2.15.0 (+50 patches)" style of versions, it'd just be "2.15.0".

Things that definitely need doing first:
 - Sort out endianness detection (maybe could use the 'cpu' package),
   but performance could be a concern.
 - Look through the other defines in Setup.hs (MAPI?? Do we care??)
 - Figure out how this interacts with our release process, particularly
   the manpage generate step.


1 patch for repository darcs-unstable@darcs.net:screened:

patch 85f9315a962c08090081d6d301d42ef9ed352bd5
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Mon Sep  2 23:43:33 BST 2019
  * WIP: drop custom Setup.hs
Attachments
msg21330 (view) Author: bfrk Date: 2019-09-03.08:31:17
> The custom Setup.hs in darcs is a source of endless trouble.

True.

> We are or have been doing things that are deprecated, it complicates
> dependency solving, and the generated files like release/distributed-context
> often cause me trouble with file locking on Windows.

We also cannot currently use darcs as a development sub-package without
ad-hoc changes to Setup.hs which complicates development of darcsden
(and other tools that depend on darcs-lib). This is due to the man page
generation which is currently done for the whole package instead of just
for darcs-exe. Unfortunately the cabal API so horribly complex that I
wasn't able to get this fixed and gave up.

> We've simplified a few bits piecemeal over the years but maybe it's time
> just take one big leap and drop it entirely.

A bold move. It might be a good idea to talk to package maintainers for
Linux distros and ask them which parts they actually use.

> One big thing that we would definitely lose is darcs --exact-version.
> This introspects on the repo at build time to embed the full patch
> context + weak hash into the binary. We also wouldn't get the
> "2.15.0 (+50 patches)" style of versions, it'd just be "2.15.0".

A while ago I opened https://github.com/haskell/cabal/issues/6180 to get
clarification about why we get the warnings when compiling Setup.hs. The
second sentence of the reply reads

"One workaround is to write a small script that will generate a `.hs`
module with the revision info and then call `sdist`, and then use this
script instead of `sdist` for creating tarballs."

If that's the way to go, then we don't need a custom Setup.hs to do that.

> Things that definitely need doing first:
>  - Sort out endianness detection (maybe could use the 'cpu' package),
>    but performance could be a concern.

I must look up why we need that at all. We shouldn't.

>  - Look through the other defines in Setup.hs (MAPI?? Do we care??)

If we want darcs to work seamlessly on Windows, we should probably care.
No idea why this needs support form Setup.hs, though. It should just be
a build-depends, conditional on Windows, perhaps supported by separating
out the "send an email" functionality to an extra module. I will take a
look.

>  - Figure out how this interacts with our release process, particularly
>    the manpage generate step.

I guess providing a separate script (e.g. in contrib) would be fine for
that.

Cheers
Ben
msg21332 (view) Author: ganesh Date: 2019-09-03.09:38:40
> We also cannot currently use darcs as a development sub-package without
> ad-hoc changes to Setup.hs which complicates development of darcsden
> (and other tools that depend on darcs-lib). This is due to the man page
> generation which is currently done for the whole package instead of just
> for darcs-exe. Unfortunately the cabal API so horribly complex that I
> wasn't able to get this fixed and gave up.

I haven't worked on darcsden for a while, but I've always used it
against darcs without any changes to Setup.hs that I remember. Is this a
recent problem?

>> We've simplified a few bits piecemeal over the years but maybe it's time
>> just take one big leap and drop it entirely.
> 
> A bold move. It might be a good idea to talk to package maintainers for
> Linux distros and ask them which parts they actually use.

Aren't they just going to be running it and packaging the output? Apart
from the manpage, the changes we would make would be only relevant to us
as developers.

>> [re darcs --exact-version]

> "One workaround is to write a small script that will generate a `.hs`
> module with the revision info and then call `sdist`, and then use this
> script instead of `sdist` for creating tarballs."
> 
> If that's the way to go, then we don't need a custom Setup.hs to do that.

I don't think we should care about --exact-version at all with sdist. We
run sdist when we are making a release, and our releases are tagged, so
we can trivially identify the repository state from the version number
if we didn't screw up the release process.

The question for me is whether we care about it the rest of the time.
Personally I don't, at least not enough to pay the price of a custom
Setup.hs.

>>  - Sort out endianness detection (maybe could use the 'cpu' package),
>>    but performance could be a concern.
> 
> I must look up why we need that at all. We shouldn't.

It's in Darcs.Util.Index. Not sure of the details.

>>  - Look through the other defines in Setup.hs (MAPI?? Do we care??)
> 
> If we want darcs to work seamlessly on Windows, we should probably care.

I use Windows and I've never used MAPI, at least not knowingly.

It would be quite nice to be able to send patches directly from Windows
though, so perhaps I should care more :-) At the moment I push them to a
Linux machine and send from there. But my long-term view is that we
should be sending things over HTTP to darcsden rather than investing
lots of effort in the email interface.

> No idea why this needs support form Setup.hs, though. It should just be
> a build-depends, conditional on Windows, perhaps supported by separating
> out the "send an email" functionality to an extra module. I will take a
> look.

Agreed we should be able to move that kind of logic into the cabal file
if we need it at all. I'd have to do more research to know whether MAPI
is actually worthwhile at all though.
msg21342 (view) Author: bfrk Date: 2019-09-03.16:09:23
>> We also cannot currently use darcs as a development sub-package without
>> ad-hoc changes to Setup.hs which complicates development of darcsden
>> (and other tools that depend on darcs-lib). This is due to the man page
>> generation which is currently done for the whole package instead of just
>> for darcs-exe. Unfortunately the cabal API so horribly complex that I
>> wasn't able to get this fixed and gave up.
> 
> I haven't worked on darcsden for a while, but I've always used it
> against darcs without any changes to Setup.hs that I remember. Is this a
> recent problem?

I meant using darcs as a "local package" as described here:

https://cabal.readthedocs.io/en/latest/nix-local-build.html#specifying-the-local-packages

This has never worked without disabling the custom build of the man page.

>>> We've simplified a few bits piecemeal over the years but maybe it's time
>>> just take one big leap and drop it entirely.
>>
>> A bold move. It might be a good idea to talk to package maintainers for
>> Linux distros and ask them which parts they actually use.
> 
> Aren't they just going to be running it and packaging the output? Apart
> from the manpage, the changes we would make would be only relevant to us
> as developers.

I have no idea what features they use. Asking doesn't hurt, does it?

>>> [re darcs --exact-version]
> 
>> "One workaround is to write a small script that will generate a `.hs`
>> module with the revision info and then call `sdist`, and then use this
>> script instead of `sdist` for creating tarballs."
>>
>> If that's the way to go, then we don't need a custom Setup.hs to do that.
> 
> I don't think we should care about --exact-version at all with sdist. We
> run sdist when we are making a release, and our releases are tagged, so
> we can trivially identify the repository state from the version number
> if we didn't screw up the release process.

Okay, that makes sense.

> The question for me is whether we care about it the rest of the time.
> Personally I don't, at least not enough to pay the price of a custom
> Setup.hs.

I think we as developers know how to identify the version we run
manually. So yes, this is perhaps no longer needed.

>>>  - Sort out endianness detection (maybe could use the 'cpu' package),
>>>    but performance could be a concern.
>>
>> I must look up why we need that at all. We shouldn't.
> 
> It's in Darcs.Util.Index. Not sure of the details.

Aargh, the index rears its ugly head. The details are pretty much that
we use mmapByteString on the file to read and edit it and that it stores
integers in binary form. This needs to aware of endianness.

>>>  - Look through the other defines in Setup.hs (MAPI?? Do we care??)
>>
>> If we want darcs to work seamlessly on Windows, we should probably care.
> 
> I use Windows and I've never used MAPI, at least not knowingly.
> 
> It would be quite nice to be able to send patches directly from Windows
> though, so perhaps I should care more :-) At the moment I push them to a
> Linux machine and send from there.

That is horrible. I can't believe there is no MAPI library on hackage
that we could use.

> But my long-term view is that we
> should be sending things over HTTP to darcsden rather than investing
> lots of effort in the email interface.

Absolutely. And I wasn't proposing to invest lots of effort, just take a
look, see what libraries are there and use one if we find it suitable.

>> No idea why this needs support form Setup.hs, though. It should just be
>> a build-depends, conditional on Windows, perhaps supported by separating
>> out the "send an email" functionality to an extra module. I will take a
>> look.
> 
> Agreed we should be able to move that kind of logic into the cabal file
> if we need it at all. I'd have to do more research to know whether MAPI
> is actually worthwhile at all though.

Yup. Will look, too.
msg21344 (view) Author: ganesh Date: 2019-09-03.16:32:37
> I meant using darcs as a "local package" as described here:
> 
> https://cabal.readthedocs.io/en/latest/nix-local-build.html#specifying-the-local-packages
> 
> This has never worked without disabling the custom build of the man page.

Interesting, I'm fairly sure I've done it without disabling that. But I
can't remember the details now. If we are going to keep Setup.hs for a
long time I can check.

[distros]
> I have no idea what features they use. Asking doesn't hurt, does it?

I guess not. What distros do we care about: Debian, NixOS, ...?

> That is horrible. I can't believe there is no MAPI library on hackage
> that we could use.

>> Agreed we should be able to move that kind of logic into the cabal file
>> if we need it at all. I'd have to do more research to know whether MAPI
>> is actually worthwhile at all though.

Wikipedia suggests it's only used with Exchange servers in practice, or
maybe the other Microsoft email clients. But
http://kb.mozillazine.org/MAPI_Support suggests that Thunderbird does
have MAPI support, albeit possibly buggy, so maybe it is somewhat
portable. Personally I use Thunderbird.

Looks like Win32 supports it:

http://hackage.haskell.org/package/Win32-2.2.0.0/docs/System-Win32-SimpleMAPI.html

And in theory we already have our own C bindings in
src/win32/send_email{.h,c}.

I guess you're not in a good position to try this out though :-) I'll
have a bit of a play.
msg21347 (view) Author: ganesh Date: 2019-09-03.17:55:54
[MAPI]
> I guess you're not in a good position to try this out though :-) I'll
> have a bit of a play.

Turns out it just works! No special options needed other than --mail. I
did get a slightly confusing popup from Outlook (which isn't really
installed but there might be some stub that comes with Office), but then
Thunderbird sent the mail.

Anyway, it should be trivial to move the logic for enabling this into
darcs.cabal. And a possible future enhancement is to move to using the
Win32 package instead of our own bindings.
msg21350 (view) Author: bfrk Date: 2019-09-03.18:05:02
> [MAPI]
>> I guess you're not in a good position to try this out though :-) I'll
>> have a bit of a play.
> 
> Turns out it just works! No special options needed other than --mail. I
> did get a slightly confusing popup from Outlook (which isn't really
> installed but there might be some stub that comes with Office), but then
> Thunderbird sent the mail.
> 
> Anyway, it should be trivial to move the logic for enabling this into
> darcs.cabal. And a possible future enhancement is to move to using the
> Win32 package instead of our own bindings.

Good news indeed and I like the plan.
msg21715 (view) Author: ganesh Date: 2020-01-25.16:45:38
Latest version. Doing something about this is becoming more urgent
as Cabal 2.4.1.0 doesn't build with GHC 8.8 (MonadFail problems) and
our custom Setup.hs doesn't work with Cabal 3.0.0.0 (sdistHook).

1 patch for repository darcs-unstable@darcs.net:screened:

patch fdff8e7672fa08952c3fad02f378b01d59653394
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Sat Jan 25 12:09:59 GMT 2020
  * WIP: drop custom Setup.hs
Attachments
msg21718 (view) Author: bfrk Date: 2020-01-25.17:50:03
> Latest version. Doing something about this is becoming more urgent
> as Cabal 2.4.1.0 doesn't build with GHC 8.8 (MonadFail problems) and
> our custom Setup.hs doesn't work with Cabal 3.0.0.0 (sdistHook).

Can you summarize what we will loose if we do that? I remember we
discussed MAPI support and the (exact-)version feature. Anything else?
Attachments
msg21719 (view) Author: ganesh Date: 2020-01-25.22:16:21
On 25/01/2020 17:50, Ben Franksen wrote:

> Can you summarize what we will loose if we do that? I remember we
> discussed MAPI support and the (exact-)version feature. Anything else?

MAPI support is already dealt with - moved to the cabal file in
"move -DHAVE_MAPI from Setup.hs to darcs.cabal".

As you say we lose --exact-version.

There's also endianness checking - most critical as that's required
inside darcs itself - and the manpage generation.
msg21729 (view) Author: bfrk Date: 2020-01-26.16:42:29
> There's also endianness checking - most critical as that's required
> inside darcs itself

We can't use the cpu package as it won't build on Windows due to the use
of C99 features (stdint.h), which I believe Windows does not support.

I just sent patch1966 that removes all the endianness code, turning the
index into an architecture dependent format. An extra header field is
used to detect different endianness and declares the format invalid in
that (rather unlikely) case, causing the index to be re-created.
Attachments
msg21730 (view) Author: bfrk Date: 2020-01-26.16:47:36
> As you say we lose --exact-version.

If we throw out the sdist hooks we can build with cabal-3. In that case
I think we can keep --exact-version, it just won't work when compiling
an archive that was created with 'cabal sdist'. I don't think many
people use 'cabal sdist'.

> manpage generation.

I guess distribution packagers can (and will) call 'darcs help manpage'
in some post-install script.

There should be a clear warning in the release notes if we remove this
from Setup.hs. Ahem, do we have release notes?
Attachments
msg21805 (view) Author: bfrk Date: 2020-02-13.23:30:55
After wasting almost two full days trying in vain to find a clean
solution for the --exact-version problem I am now ready to give up. The
Cabal library is a moving target with out a stable API. This is almost
as bad as the Darcs library.

Let us throw out the custom Setup for good. It is just not worth the effort.

BTW, I think we can remove the Setup.hs, cabal no longer needs it.
Attachments
msg21810 (view) Author: bfrk Date: 2020-02-14.01:10:33
Rebased to resolve conflict & removed Setup.hs

1 patch for repository http://darcs.net/screened:

patch cfe1a7106c414f6518da1e47f4f9bb9f2d75cd6b
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Jan 25 13:09:59 CET 2020
  * WIP: drop custom Setup.hs
Attachments
History
Date User Action Args
2019-09-02 22:44:51ganeshcreate
2019-09-02 22:45:35ganeshsetstatus: needs-screening -> in-discussion
2019-09-03 08:31:18bfrksetmessages: + msg21330
2019-09-03 09:38:40ganeshsetmessages: + msg21332
2019-09-03 16:09:23bfrksetmessages: + msg21342
2019-09-03 16:32:37ganeshsetmessages: + msg21344
2019-09-03 17:55:54ganeshsetmessages: + msg21347
2019-09-03 18:05:02bfrksetmessages: + msg21350
2020-01-25 16:45:38ganeshsetfiles: + patch-preview.txt, wip_-drop-custom-setup_hs.dpatch, unnamed
messages: + msg21715
2020-01-25 17:50:03bfrksetfiles: + pEpkey.asc
messages: + msg21718
2020-01-25 22:16:21ganeshsetmessages: + msg21719
2020-01-26 16:42:29bfrksetfiles: + pEpkey.asc
messages: + msg21729
2020-01-26 16:47:37bfrksetfiles: + pEpkey.asc
messages: + msg21730
2020-02-13 23:30:55bfrksetfiles: + pEpkey.asc
messages: + msg21805
2020-02-14 01:10:33bfrksetfiles: + patch-preview.txt, wip_-drop-custom-setup_hs_0.dpatch, unnamed
messages: + msg21810