webmacs: init at version 0.8 #69088

jacg · 2019-09-19T10:45:23Z

Motivation for this change

This package was not available in nixpkgs, before.

(The check phase is disabled at present, at is depends on a number of things that are not available in nixpkgs. I plan to contribute these as well, but wanted to keep my first contribution down to a manageable and easily-reviewable size.)

Things done

Notify maintainers

cc @

alexarice

Thanks for choosing to contribute to nixpkgs, looks like an interesting program.
I've put in a few comments to bring the style closer to other packages in nixpkgs

Also at some point you will want to rebase so that you have 2 commits

The first adding yourself to the maintainers list titled maintainers: add jacg
The second adding the package titled webmacs: init at 0.8

maintainers/maintainer-list.nix

pkgs/applications/networking/browsers/webmacs/default.nix

alexarice · 2019-09-19T11:41:57Z

pkgs/applications/networking/browsers/webmacs/default.nix

+    platforms = platforms.all;
+  };
+
+  preBuild = "CC=g++";


This doesn't look right. Nix should normally handle setting up your C compiler

Hmm, yes ... it needs to use a C++ rather than a C compiler. Any hints on how else to persuade it to do so?

The build fails with gcc?

Yup:

gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/build/source/vendor/bloom-filter-cpp -I/build/source/vendor/hashset-cpp -I/build/source/vendor/ad-block -I/nix/store/96p42644i76jqgdkxjgyl3f0c8n8k16j-python3-3.7.4/include/python3.7m -c /build/source/c/adblock.c -o build/temp.linux-x86_64-3.7/build/source/c/adblock.o -g0 -std=c++11 cc1: warning: command line option ‘-std=c++11’ is valid for C++/ObjC++ but not for C /build/source/c/adblock.c:19:10: fatal error: iostream: No such file or directory #include <iostream> ^~~~~~~~~~ compilation terminated. error: command 'gcc' failed with exit status 1 builder for '/nix/store/lb8cqs8ypcsb94mj85r7k1kv1s73r51b-webmacs-0.8.drv' failed with exit code 1

... which is to be expected, when compiling C++ programs with a C compiler (unless you explicitly tell it where to find the C++ standard libraries ... but that's kinda the point of g++: it takes care of those sorts of things).

I feel it should be using $CXX but I don't know what you can do if it doesn't (apart from what you have done

What exactly do you mean by this (I'm still a little lost in nix terminology)? If I try putting CC = "g++"; at the same level as pname = "webmacs; then it fails in exactly the same way.

that is what I meant but clearly it doesn't work

leave it as is for now, It is probably me being picky about something that is actually ok

Let me poke around a bit to try to understand why it's using $CC rather than $CXX, but if I don't learn anything in a reasonably short amount of time, then I'll leave it as is. In the meantime I'll push all the other changes you requested.

Whatever the result of this, this should go a bit further up in the file and not be hidden down here

pkgs/applications/networking/browsers/webmacs/default.nix

pkgs/top-level/all-packages.nix

jacg · 2019-09-19T13:29:46Z

@alexrice Thank you very much for taking the time to look at this and provide the helpful review comments. I've pushed the rebased branch with all (I hope) the changes you requested except the CC vs CXX issue.

alexarice · 2019-09-19T13:35:42Z

Traceback (most recent call last):
  File "/nix/store/7hp7vaai1lxmrfmf0hrz078sjnzqgk72-webmacs-0.8/bin/.webmacs-wrapped", line 7, in <module>
    from webmacs.main import main
  File "/nix/store/7hp7vaai1lxmrfmf0hrz078sjnzqgk72-webmacs-0.8/lib/python3.7/site-packages/webmacs/main.py", line 27, in <module>
    from .ipc import IpcServer
  File "/nix/store/7hp7vaai1lxmrfmf0hrz078sjnzqgk72-webmacs-0.8/lib/python3.7/site-packages/webmacs/ipc.py", line 23, in <module>
    from . import version
  File "/nix/store/7hp7vaai1lxmrfmf0hrz078sjnzqgk72-webmacs-0.8/lib/python3.7/site-packages/webmacs/version.py", line 25, in <module>
    from PyQt5.QtWebEngineWidgets import QWebEngineProfile
ModuleNotFoundError: No module named 'PyQt5.QtWebEngineWidgets'

any ideas?

maintainers/maintainer-list.nix

pkgs/applications/networking/browsers/webmacs/default.nix

jacg · 2019-09-19T13:58:51Z

Hmm ... I'm not seeing that here (which is exactly the sort of thing that isn't supposed to happen with nix!).

How should I be testing this reliably, locally?

nix-build $NIXPKGS -A webmacs

certainly doesn't give me the problem you observe.

(Also, when I try to break it deliberately, for example, by changing the revision hash, it happily gives me whatever it built before, which is worrying.)

alexarice · 2019-09-19T14:00:40Z

Changing the revision hash will not work as fetchFromGitHub is a fixed output derivation. Basically if there is an output in the store that has the hash given by sha256 then it will not rebuild, even if the inputs have changed

alexarice · 2019-09-19T14:01:33Z

I get the problem after running nix-build -A webmacs from inside a nixpkgs clone and then running result/bin/webmacs from command line

jacg · 2019-09-19T14:01:49Z

$nix-shell --pure -p nix
[nix-shell:~]$ nix-build ~/src/nixpkgs/ -A webmacs
/nix/store/7hp7vaai1lxmrfmf0hrz078sjnzqgk72-webmacs-0.8

I've run it in a pure shell, it's the same hash as the one you observe ... what else can I try to reproduce the error?

alexarice · 2019-09-19T14:02:21Z

It builds fine, it is running the program which causes issue

pkgs/applications/networking/browsers/webmacs/default.nix

jacg · 2019-09-19T15:24:44Z

No module named 'PyQt5.QtWebEngineWidgets' [...] any ideas?

It looks like the use of wrapQtAppsHook has become required since I first created this package and started using it locally (and I guess that a rebase just now has made the two clash). But I'm struggling to figure out exactly how it interacts with buildPythonApplication, so I'm not making any progress.

alexarice · 2019-09-19T15:45:46Z

I cannot work this out either. I believe you want to follow the advice in https://github.com/NixOS/nixpkgs/pull/68405 because this package is very similar though this does not fix the missing library

alexarice

Found the issue, you need pyqtwebengine instead of pyqt5_with_qtwebkit
See https://stackoverflow.com/questions/51154871/python-3-7-0-no-module-named-pyqt5-qtwebenginewidgets

pkgs/applications/networking/browsers/webmacs/default.nix

jacg · 2019-09-19T16:22:42Z

One thing, before I forget: nix-shell -I nixpkgs=$NIXPKGS -p webmacs --run "webmacs webmacs://version -i \"\"" opens a webmacs which claims to have

Qt version: 5.12.3
PyQt version: 5.13.0

This is disturbing.

alexarice · 2019-09-19T16:54:57Z

This is probably lost in the mess of messages above but can the maintainer commit be called "maintainers: add jacg"

jacg · 2019-09-19T17:02:04Z

Done.

alexarice

Few more small cleanup changes but this builds and runs with nix-review

I hope this hasn't been too traumatic. Most packages are far easier to package than this.

pkgs/applications/networking/browsers/webmacs/default.nix

alexarice · 2019-09-19T17:05:24Z

pkgs/applications/networking/browsers/webmacs/default.nix

+    platforms = platforms.all;
+  };
+
+  preBuild = "CC=g++";


Whatever the result of this, this should go a bit further up in the file and not be hidden down here

alexarice · 2019-09-19T17:10:01Z

@GrahamcOfBorg build

jacg · 2019-09-19T17:19:14Z

Few more small cleanup changes [...]

Done.

I hope this hasn't been too traumatic. Most packages are far easier to package than this.

I held off submitting it for ages, fearing the process, but your help and patience has made perfectly bearable. Thank you very much.

jacg · 2019-09-19T17:21:33Z

If the CC vs CXX issue doesn't bother you too much, then I'd leave it like that: I don't have time to look at it now, and don't know when I'll find some.

alexarice · 2019-09-19T17:23:00Z

I would leave it for now. Someone else will have to take a look at this as well as I do not have commit rights. They may have an opinion on it

jacg · 2019-09-26T08:49:53Z

@alexarice I get the impression that this PR is simply adding to the growing backlog of over 1000 open PRs. If the committers are overwhelmed by the sheer volume of PRs, perhaps it's best just to close this? There are more important things to be done than getting this package in.

alexarice · 2019-09-26T08:51:52Z

I think you should leave it open. If no one gets round to looking at it then it does not burden committers and if they do get round to looking at it then it can get merged.

You could also consider making an NUR repo if you want to make it available sooner

nixos-discourse · 2019-09-26T08:53:55Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/59

alexarice · 2019-09-26T09:31:29Z

@GrahamcOfBorg build webmacs

worldofpeace · 2019-09-27T02:13:33Z

@GrahamcOfBorg build webmacs

worldofpeace · 2019-09-27T02:16:26Z

@alexarice I get the impression that this PR is simply adding to the growing backlog of over 1000 open PRs. If the committers are overwhelmed by the sheer volume of PRs, perhaps it's best just to close this? There are more important things to be done than getting this package in.

@alexarice Is correct on this @jacg, eventually a committer will get around to looking at your change. Them actually asking in discourse helped grab my attention at least.

This particular package has been reviewed very well, and I've pushed any nitpicks and made CC=$CXX as that's actually the variable they should be using.

ofborg bot added 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Sep 19, 2019

alexarice reviewed Sep 19, 2019

View reviewed changes

jacg force-pushed the webmacs branch from 1c290f9 to c5aeb48 Compare September 19, 2019 13:27

alexarice reviewed Sep 19, 2019

View reviewed changes

pkgs/applications/networking/browsers/webmacs/default.nix Outdated Show resolved Hide resolved

alexarice reviewed Sep 19, 2019

View reviewed changes

jacg force-pushed the webmacs branch 2 times, most recently from b4af821 to ef850cc Compare September 19, 2019 16:53

jacg force-pushed the webmacs branch from ef850cc to cdce5e8 Compare September 19, 2019 17:01

alexarice approved these changes Sep 19, 2019

View reviewed changes

jacg force-pushed the webmacs branch from cdce5e8 to 4205358 Compare September 19, 2019 17:16

maintainers: add jacg

485adfa

jacg force-pushed the webmacs branch from 4205358 to 4f6cbd2 Compare September 26, 2019 08:50

worldofpeace changed the title ~~New package: webmacs~~ webmacs: init at version 0.8 Sep 27, 2019

webmacs: init at version 0.8

bcecdd5

worldofpeace force-pushed the webmacs branch from 3bd3b02 to bcecdd5 Compare September 27, 2019 02:44

worldofpeace merged commit c9b85da into NixOS:master Sep 27, 2019

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 0 and removed 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Sep 27, 2019

Janik-Haag added the 12. first-time contribution label Jun 12, 2023

webmacs: init at version 0.8 #69088

webmacs: init at version 0.8 #69088

Conversation

jacg commented Sep 19, 2019

Motivation for this change

Things done

Notify maintainers

alexarice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacg commented Sep 19, 2019

alexarice commented Sep 19, 2019

jacg commented Sep 19, 2019

alexarice commented Sep 19, 2019

alexarice commented Sep 19, 2019

jacg commented Sep 19, 2019

alexarice commented Sep 19, 2019

jacg commented Sep 19, 2019

alexarice commented Sep 19, 2019

alexarice left a comment

Choose a reason for hiding this comment

jacg commented Sep 19, 2019

alexarice commented Sep 19, 2019

jacg commented Sep 19, 2019

alexarice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexarice commented Sep 19, 2019

jacg commented Sep 19, 2019

jacg commented Sep 19, 2019

alexarice commented Sep 19, 2019

jacg commented Sep 26, 2019

alexarice commented Sep 26, 2019

nixos-discourse commented Sep 26, 2019

alexarice commented Sep 26, 2019

worldofpeace commented Sep 27, 2019

worldofpeace commented Sep 27, 2019