astroid: 0.11.1 -> 0.13 #44226

yrashk · 2018-07-30T10:49:18Z

Couple of things were updated to make this work.

webkitgtk24x-gtk3 that is marked unsafe has been replaced with
webkitgtk (2.20.*) as astroid has recently indicated that >= 2.20
is
acceptable
protobuf built input was added to satisfy requirements
astroid was not functional at all without proper icons so gnome3.defaultIconTheme
were added to buildInputs
By default, astroid will use gvim as a setting for the embedded
editor, however that didn't work well with nixpkgs. vim is now
bundled with astroid by default. This setting can be overridden
by the user in astroid's config file.

Motivation for this change

Astroid 0.11.1 is outdated

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
Built on platform(s)
- 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 nox --run "nox-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)
Fits CONTRIBUTING.md.

xeji · 2018-07-30T11:10:11Z

@GrahamcOfBorg build astroid

GrahamcOfBorg · 2018-07-30T11:28:50Z

Success on aarch64-linux (full log)

Attempted: astroid

Partial log (click to expand)

post-installation fixup
Wrapping program /nix/store/kiffy1h5a3ywxvahysbf1xd54fcydryy-astroid-0.13/bin/astroid
shrinking RPATHs of ELF executables and libraries in /nix/store/kiffy1h5a3ywxvahysbf1xd54fcydryy-astroid-0.13
shrinking /nix/store/kiffy1h5a3ywxvahysbf1xd54fcydryy-astroid-0.13/bin/.astroid-wrapped
shrinking /nix/store/kiffy1h5a3ywxvahysbf1xd54fcydryy-astroid-0.13/lib/astroid/web-extensions/libtvextension.so
strip is /nix/store/zrs21zqcchgyabjf4xfimncdq16njizc-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/kiffy1h5a3ywxvahysbf1xd54fcydryy-astroid-0.13/lib  /nix/store/kiffy1h5a3ywxvahysbf1xd54fcydryy-astroid-0.13/bin
patching script interpreter paths in /nix/store/kiffy1h5a3ywxvahysbf1xd54fcydryy-astroid-0.13
checking for references to /build in /nix/store/kiffy1h5a3ywxvahysbf1xd54fcydryy-astroid-0.13...
/nix/store/kiffy1h5a3ywxvahysbf1xd54fcydryy-astroid-0.13

yrashk · 2018-07-30T11:33:11Z

Turns out there's still a problem with email composition, it loses the body of the message. Trying to figure out if this is Nixpkgs-related or not.

teto · 2018-07-30T11:48:38Z

Really happy to see a PR but can't we make the vim dependency optional ? astroid can be used with any editor last time I tried.

GrahamcOfBorg · 2018-07-30T11:48:48Z

Success on x86_64-linux (full log)

Attempted: astroid

Partial log (click to expand)

post-installation fixup
Wrapping program /nix/store/j2zm0g5k7iwvq6ccn4hd0rabacnfqch0-astroid-0.13/bin/astroid
shrinking RPATHs of ELF executables and libraries in /nix/store/j2zm0g5k7iwvq6ccn4hd0rabacnfqch0-astroid-0.13
shrinking /nix/store/j2zm0g5k7iwvq6ccn4hd0rabacnfqch0-astroid-0.13/lib/astroid/web-extensions/libtvextension.so
shrinking /nix/store/j2zm0g5k7iwvq6ccn4hd0rabacnfqch0-astroid-0.13/bin/.astroid-wrapped
strip is /nix/store/1hi76hr87bd1y1q1qjk0lv8nmcjip1c8-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/j2zm0g5k7iwvq6ccn4hd0rabacnfqch0-astroid-0.13/lib  /nix/store/j2zm0g5k7iwvq6ccn4hd0rabacnfqch0-astroid-0.13/bin
patching script interpreter paths in /nix/store/j2zm0g5k7iwvq6ccn4hd0rabacnfqch0-astroid-0.13
checking for references to /build in /nix/store/j2zm0g5k7iwvq6ccn4hd0rabacnfqch0-astroid-0.13...
/nix/store/j2zm0g5k7iwvq6ccn4hd0rabacnfqch0-astroid-0.13

yrashk · 2018-07-30T12:50:55Z

@teto it can, indeed -- I just wanted to include it because without it, by default, astroid won't be functional. Should I make it optional but enabled by default?

GrahamcOfBorg · 2018-07-30T14:35:37Z

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: astroid

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

Couple of things were updated to make this work. 1. webkitgtk24x-gtk3 that is marked unsafe has been replaced with webkitgtk (2.20.*) as astroid has recently indicated that [>= 2.20 is acceptable](astroidmail/astroid@48ce7e3) 2. protobuf built input was added to satisfy requirements 3. astroid was not functional at all without proper icons so gnome3.defaultIconTheme were added to buildInputs 4. By default, astroid will use `gvim` as a setting for the embedded editor, however that didn't work well with nixpkgs. vim is now bundled with astroid by default. This setting can be overridden by the user in astroid's config file.

yrashk · 2018-08-08T07:14:59Z

I've updated this PR to make vim optional at a build time (enabled by default).

Also, the issue with composition feature is only happening in certain configurations, and is not Nixpkgs-specific. It's been acknowledged and is being worked on astroidmail/astroid#532. It only affects some users.

teto · 2018-08-09T02:54:09Z

I could build it locally but it crashed on startup

[2018-08-09 11:44:09.865693] [0x00007ffb98896a00] [info]    cf: loading: "/home/teto/.config/astroid/config"
[2018-08-09 11:44:09.865723] [0x00007ffb98896a00] [warning] cf: making runtime dir..
[2018-08-09 11:44:09.866115] [0x00007ffb98896a00] [info]    cf: version: 11
[2018-08-09 11:44:09.866211] [0x00007ffb98896a00] [debug]   cf: check config..
[2018-08-09 11:44:09.866225] [0x00007ffb98896a00] [error]   cf: the config file is an old version (9), the current version is: 11
[2018-08-09 11:44:09.866229] [0x00007ffb98896a00] [warning] cf: missing values in config have been updated with defaults (old version: 9, new: 11)
[11:44:09] [0x00007ffb98896a00] [M] [info] welcome to astroid! - 0.13.0
[11:44:09] [0x00007ffb98896a00] [M] [info] date: init.
[11:44:09] [0x00007ffb98896a00] [M] [info] db path: /home/teto/Maildir
terminate called after throwing an instance of 'boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::property_tree::ptree_bad_data> >'
  what():  conversion of data to type "b" failed
zsh: abort      result/bin/astroid

anyway, I am still reluctant in having vim as a default dependency. Can't astroid use the value of EDITOR instead ? in worst case, it won't launch the editor and print a warning. If you insist in having nix manage the configuration, I recommand to look at the new home-manager infrastructure for mails, some cool stuff is happening nix-community/home-manager#318

yrashk · 2018-08-09T02:58:38Z

Not sure about the crash, I think this one is worth reporting in the astroid repo.

As for EDITOR, we can make a patch for that, but it won't help with all the arguments it is passing to gvim. If we just switch this to EDITOR, I believe, often times, the end-user will be getting a mail client that can't compose e-mails at all.

What are the main concerns about using vim by default to support astroid's default configuration? It can be turned off or replaced with a different dependency if the user wishes so.

yrashk · 2018-08-09T03:00:14Z

Nix doesn't manage the configuration per se in this case, but just sets working defaults for astroid to produce when the new configuration file is produced.

If this concern is a roadblock, I can, of course, remove this dependency altogether (or disable it by default), but I think it'll make it for a much more confusing initial experience ("why does e-mail composition not work?")

tkerber · 2018-08-09T10:52:42Z

I don't see that much of an issue with vim being a default dependency, and I do think there is value in having functional default behaviour. One thing I'm not sure about is the specific version of vim? I'd also note that EDITOR is often set to a terminal-only editor, in which case it's non-trivial to have sensible behaviour, as it would have to a) decide if EDITOR is graphical or not, and b) if not, decide which terminal emulator to launch it in.

If this continues to be a concern, then maybe split the PR into a version update and adding vim as a dependency, so that at least the security-critical update can be used?

teto · 2018-08-09T12:08:02Z

I am no nixpkgs maintainer so feel free to ignore my comments :) My experience with nixpkgs is that maintainers do pay attention to closure size and while vim isn't huge it's not light either (also depends which one, if you pull gvim then ouch) I don't use vim but changing the astroid package would imply recompiling astroid (which I would rather avoid for this kind of medium-sized application). For people using vim, vim would be in PATH most likely so it should work out of the box, for the others they would change the setting anyway. Astroid needs a fair bit of configuration anyway (notmuch, poll mechanisms etc) so I am not too concerned about the out of the box experience. It would be realy improved with home-manager integration. I've said too much, let's hope a nixos member chimes in. 2018-08-09 19:52 GMT+09:00 Thomas Kerber <notifications@github.com>:

…

I don't see that much of an issue with vim being a default dependency, and I do think there is value in having functional default behaviour. One thing I'm not sure about is the specific version of vim? I'd also note that EDITOR is often set to a terminal-only editor, in which case it's non-trivial to have sensible behaviour, as it would have to a) decide if EDITOR is graphical or not, and b) if not, decide which terminal emulator to launch it in. If this continues to be a concern, then maybe split the PR into a version update and adding vim as a dependency, so that at least the security-critical update can be used? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#44226 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA2FOqRV0IwEkxJguJXMC7ifnJeVqylQks5uPBSAgaJpZM4VmMOM> .

yrashk · 2018-08-13T03:11:16Z

what is the best way to reach a resolution on this issue?

yrashk · 2018-08-19T01:54:09Z

What should I do with this PR?

globin · 2018-08-22T00:20:42Z

I think this is fine for now and can be improved further at a later point! Definitely nice to remove that ancient webkitgtk dependency here!

GrahamcOfBorg added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Jul 30, 2018

yrashk force-pushed the astroid-0.13 branch from 58e1109 to a01986a Compare August 8, 2018 07:13

oxzi mentioned this pull request Aug 10, 2018

astroid: 0.11.1 -> 0.13 #44873

Closed

9 tasks

globin self-assigned this Aug 13, 2018

globin merged commit 61c0adf into NixOS:master Aug 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

astroid: 0.11.1 -> 0.13 #44226

astroid: 0.11.1 -> 0.13 #44226

yrashk commented Jul 30, 2018

xeji commented Jul 30, 2018

GrahamcOfBorg commented Jul 30, 2018

yrashk commented Jul 30, 2018

teto commented Jul 30, 2018

GrahamcOfBorg commented Jul 30, 2018

yrashk commented Jul 30, 2018 •

edited

GrahamcOfBorg commented Jul 30, 2018

yrashk commented Aug 8, 2018

teto commented Aug 9, 2018 •

edited

yrashk commented Aug 9, 2018

yrashk commented Aug 9, 2018 •

edited

tkerber commented Aug 9, 2018

teto commented Aug 9, 2018 via email •

edited

yrashk commented Aug 13, 2018

yrashk commented Aug 19, 2018

globin commented Aug 22, 2018

astroid: 0.11.1 -> 0.13 #44226

astroid: 0.11.1 -> 0.13 #44226

Conversation

yrashk commented Jul 30, 2018

Motivation for this change

Things done

xeji commented Jul 30, 2018

GrahamcOfBorg commented Jul 30, 2018

yrashk commented Jul 30, 2018

teto commented Jul 30, 2018

GrahamcOfBorg commented Jul 30, 2018

yrashk commented Jul 30, 2018 • edited

GrahamcOfBorg commented Jul 30, 2018

yrashk commented Aug 8, 2018

teto commented Aug 9, 2018 • edited

yrashk commented Aug 9, 2018

yrashk commented Aug 9, 2018 • edited

tkerber commented Aug 9, 2018

teto commented Aug 9, 2018 via email • edited

yrashk commented Aug 13, 2018

yrashk commented Aug 19, 2018

globin commented Aug 22, 2018

yrashk commented Jul 30, 2018 •

edited

teto commented Aug 9, 2018 •

edited

yrashk commented Aug 9, 2018 •

edited

teto commented Aug 9, 2018 via email •

edited