neomutt: minor cleanup changes #30401
Conversation
|
Looks like the update was already merged in PR #30394. |
|
Indeed. I'll change this into a cleanup PR then instead. Thanks for noticing @gebner ! |
peterhoeg
changed the title
|
|
||
| enableParallelBuilding = true; | ||
| postInstall = '' | ||
| ln -s $out/bin/neomutt $out/bin/mutt |
There was a problem hiding this comment.
Was this decision intentionally? Any other linux distribution making this symlink?
There was a problem hiding this comment.
Well, it was done intentionally by me! ;-)
I have no idea if others are doing it, but if we apply the principle of least surprise we shouldn't just break calling mutt.
There was a problem hiding this comment.
I agree that surprise is bad in this case. I am not yet convinced of a situation where I will be using mutt and neomutt on the same machine and thus would not want that symlink... Neomutt should be pretty much capable of doing whatever mutt does (more or less).
While I prefer having the compatibility (for now) how would you go about this in the long term?
As far as I can see ArchLinux (AUR) is still lacking any kind of action regarding the latest release. Gentoo has been moving the neomutt naming for a while now (https://gitweb.gentoo.org/repo/gentoo.git/tree/mail-client/neomutt). As far as I can tell they do not maintain any backwards compatibility in this case.
I think introducing neomutt as the name of the binary early is a good thing. (As has been done here since it is just a symlink)
Keeping mutt as an alias around on the long run is debatable. For the purpose of keeping legacy craft small dropping it (at some point) is probably good.
Is there any kind of deprecation warning we can produce when someone is calling mutt instead of neomutt? Making the mutt (binary / script) a wrapper around neomutt that spits out a warning?
There was a problem hiding this comment.
Archlinux now has a package in community-testing (before, only a PKGBUILD in AUR).
They also don't add a symlink or wrapper for bin/mutt there.
I'd agree a wrapper issuing a deprecation warning might be something useful for 17.09, but simply a symlink is not enough IMHO, as users don't know they need to upgrade to the new name.
There was a problem hiding this comment.
Seems like the patch doesn't apply cleanly anymore :-/
Also, is there a reason you removed enableParallelBuilding = true;?
Additionally, neomutt recently added an Autosetup-based build system. Maybe we should also move to using this? (in a followup PR probably)…
There was a problem hiding this comment.
Also, is there a reason you removed enableParallelBuilding = true;?
I didn't remove it - just moved it together with the other "build" options.
I'll rebase tonight against master.
There was a problem hiding this comment.
Ah, right - found it. Thanks!
There was a problem hiding this comment.
Is everybody OK with the wrapper?
- neomutt has changed the binary name so we enable "mutt" via a wrapper script with a warning - we don't have to manually run the prepare.sh script as autoreconfHook handles everything for us - doc generation was previously trying to fetch DTDs from the net
A few other minor changes:
everything for us
Cc: @cstrahan @erikryb @jfrankenau @vrthra
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)