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 2019-09-03.18:05:02 by bf.

Files
File name Status Uploaded Type Edit Remove
patch-preview.txt ganesh, 2019-09-02.22:44:51 text/x-darcs-patch
unnamed ganesh, 2019-09-02.22:44:51 text/plain
wip_-drop-custom-setup_hs.dpatch ganesh, 2019-09-02.22:44:51 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: bf 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: bf 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: bf 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.
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:18bfsetmessages: + msg21330
2019-09-03 09:38:40ganeshsetmessages: + msg21332
2019-09-03 16:09:23bfsetmessages: + msg21342
2019-09-03 16:32:37ganeshsetmessages: + msg21344
2019-09-03 17:55:54ganeshsetmessages: + msg21347
2019-09-03 18:05:02bfsetmessages: + msg21350