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

Jump to bottom

emacspeak: init at 51.0 #73957

Merged

Mic92 merged 2 commits into NixOS:master from Dema:emacspeak

May 1, 2020

Contributor

Dema commented Nov 23, 2019

Closes #70261

Motivation for this change

New package

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
Built on platform(s)
- [ x] NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Determined the impact on package closure size (by running nix path-info -S before and after)
Ensured that relevant documentation is up to date
Fits CONTRIBUTING.md.

Needs to be tested by users of emacspeak. There're a lot of irrelevant files in the sources like blog posts, speeches, radio playlists and so on. I copied what I think is needed to run it. But I don't know how to use emacspeak, so we need some real user to test if it works.

Notify maintainers

I didn't set meta.maintainers field

ofborg bot added 8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels

Member

adisbladis commented Nov 23, 2019 •

edited

Thank you for your contribution!

Emacs packages should go in the emacs package set, this one should go in https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/editors/emacs-modes/manual-packages.nix.

This is not only an organisational thing but also so that functions like emacsWithPackages can pick it up.

jonringer reviewed

View reviewed changes

pkgs/applications/audio/emacspeak/default.nix Outdated Show resolved Hide resolved

pkgs/applications/audio/emacspeak/default.nix Outdated

Comment on lines 32 to 34

    
                cd servers/native-espeak

                make DESTDIR=$out install

                cd ../../

Contributor

jonringer Nov 24, 2019

This seems odd. Do you have to call a specific Makefile?

makeFlags = [ "DESTDIR=${placeholder "out"}" ];

Contributor Author

Dema Nov 24, 2019 •

edited

Yeah, there's a separate Makefile that builds tclespeak.so. Main Makefile doesn't do install. It just prints message to add current folder to emacs config. Could you explain a bit about placeholder? $out is deprecated?

Contributor

jonringer Nov 24, 2019

in most situations, there's no difference. From what I understand, placeholder just ensures that a variable doesn't get resolved until the last possible second. I don't remember the exact scenarios in which it was necessary to add the placeholder feature tbh

Contributor

jonringer Nov 24, 2019

but these three lines could be condensed to:

make -C servers/native-espeak DESTDIR=$out install

Dema force-pushed the emacspeak branch from 334edb3 to 0c79412 Compare

November 24, 2019 07:34

Dema requested a review from adisbladis as a code owner

November 24, 2019 07:34

Contributor Author

Dema commented Nov 24, 2019

Moved callPackage to applications/editors/emacs-modes/manual-packages.nix but left default.nix in applications/audio, is it ok?

ofborg bot added 6.topic: emacs 10.rebuild-linux: 0 and removed 10.rebuild-linux: 1-10 labels

Member

adisbladis commented Nov 24, 2019

Please also move the expression itself.

Dema force-pushed the emacspeak branch from 0c79412 to 4eae671 Compare

November 24, 2019 19:43

Contributor Author

Dema commented Nov 24, 2019

done!

jonringer reviewed

View reviewed changes

pkgs/applications/editors/emacs-modes/emacspeak/default.nix Outdated

Comment on lines 26 to 27

    
                make espeak

              '';

Contributor

jonringer Nov 24, 2019

fix alignment please

pkgs/applications/editors/emacs-modes/emacspeak/default.nix Outdated

    
                  sha256 = "043mn3rr48gx2mi4rnkcvp09fwhs88yyqcszavkzvvd57mh4vd0h";

                };

                nativeBuildInputs = [rsync gnused] ;

Contributor

jonringer Nov 24, 2019

Suggested change

      
              nativeBuildInputs = [rsync gnused] ;
          
              nativeBuildInputs = [ rsync gnused ] ;

pkgs/applications/editors/emacs-modes/emacspeak/default.nix Outdated

    
                  homepage = https://github.com/tvraman/emacspeak/;

                  description = "Emacs extension that provides spoken output";

                  license = licenses.gpl2;

                  maintainers = [];

Contributor

jonringer Nov 24, 2019

do you mind adding yourself as a maintainer? mostly just so you get notified if this gets bumped. Then you can verify that the software still works.

Dema force-pushed the emacspeak branch from 4eae671 to 17ad5dc Compare

November 25, 2019 20:31

Dema force-pushed the emacspeak branch from 17ad5dc to 043ece1 Compare

December 10, 2019 22:24

Dema force-pushed the emacspeak branch from 043ece1 to 375e4f0 Compare

April 30, 2020 13:56

Contributor Author

Dema commented Apr 30, 2020

Could you, please re-review PR. I got back to nixos after rage quitting it last autumn after fighting with nix language. But it's too attractive in it's core so I decided to return again :-)

Mic92 reviewed

View reviewed changes

pkgs/applications/editors/emacs-modes/emacspeak/default.nix Outdated

    
                nativeBuildInputs = [ rsync gnused ];

                buildInputs = [ emacs tcl tclx espeak-ng ];

                postPatch = ''

                  substituteInPlace servers/native-espeak/Makefile --replace '/usr/include/tcl$(TCL_VERSION)' ${tcl}/include

Member

Mic92 Apr 30, 2020

You can overwrite both setting:

makeFlags = [ "TCL_INCLUDE=${tcl}/include" "LIBPARENTDIR = /" ];

Mic92 reviewed

View reviewed changes

pkgs/applications/editors/emacs-modes/emacspeak/default.nix Outdated

    
                '';

                postBuild = ''

                  make espeak

Member

Mic92 Apr 30, 2020 •

edited

makeFlags won't be effective here, but make espeak just calls servers/native-espeak/Makefile. So you can also do this:

make -C servers/native-espeak "TCL_INCLUDE=${tcl}/include" "LIBPARENTDIR=/"

Mic92 reviewed

View reviewed changes

pkgs/applications/editors/emacs-modes/emacspeak/default.nix Outdated

    
                  sha256 = "06azq2566pj4pnyawqm9387ybp3wr1pr6828hhdxhki4dpn9pj8g";

                };

                nativeBuildInputs = [ rsync gnused ];

Member

Mic92 Apr 30, 2020

sed is already in stdenv.

Suggested change

      
              nativeBuildInputs = [ rsync gnused ];
          
              nativeBuildInputs = [ rsync ];

Mic92 reviewed

View reviewed changes

pkgs/applications/editors/emacs-modes/emacspeak/default.nix Outdated

    
                };

                nativeBuildInputs = [ rsync gnused ];

                buildInputs = [ emacs tcl tclx espeak-ng ];

Member

Mic92 Apr 30, 2020 •

edited

If emacs is used during the build for byte-compiling, it should be in nativeBuildInputs.

Mic92 reviewed

View reviewed changes

pkgs/applications/editors/emacs-modes/emacspeak/default.nix Outdated

    
                src = fetchFromGitHub {

                  owner = "tvraman";

                  repo = "emacspeak";

Member

Mic92 Apr 30, 2020

In Archlinux they seem to use the release tarball.
This should contain less irrelevant files:

https://github.com/tvraman/emacspeak/blob/master/Makefile#L74
https://github.com/tvraman/emacspeak/releases/download/51.0/emacspeak-51.0.tar.bz2
https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=emacspeak

Dema force-pushed the emacspeak branch from 375e4f0 to f78eabe Compare

April 30, 2020 18:12

Contributor Author

Dema commented Apr 30, 2020

Updated as per suggestions and took installation code from arch package :), looks a bit cleaner.


          emacspeak: init at 51.0

e43ad6e

Closes NixOS#70261

Mic92 requested changes

View reviewed changes

pkgs/applications/editors/emacs-modes/emacspeak/default.nix Outdated Show resolved Hide resolved

pkgs/applications/editors/emacs-modes/emacspeak/default.nix Outdated Show resolved Hide resolved

Dema force-pushed the emacspeak branch from 09aa9df to e43ad6e Compare

April 30, 2020 19:36

Contributor Author

Dema commented Apr 30, 2020

Forgot to push even more clean version :)

Dema changed the title ~~emacspeak: init at 50.0~~ emacspeak: init at 51.0


          emacspeak: set PREFIX rather than DESTDIR

675a89f

This usually causes less problems w.r.t. hard-codes paths.

Member

Mic92 commented May 1, 2020 •

edited

This was working for me now:

$ nix-shell -p emacsPackages.emacspeak --run "emacspeak"

I heard espeak reading out emacs.

Mic92 merged commit 170f301 into NixOS:master

Dema deleted the emacspeak branch

May 2, 2020 08:39

Member

adisbladis commented May 12, 2020

The meta attribute in this derivation was wrong, fixed in a2ad2ba.

Also @Dema it seems like you are not in the maintainers list. If you want to maintain packages in nixpkgs please add yourself to the maintainers list & re-add yourself to the derivation in a follow-up PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment