Skip to content
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

libxml2 and libxslt: build against python3 by default #63174

Merged
merged 5 commits into from Oct 29, 2019
Merged

libxml2 and libxslt: build against python3 by default #63174

merged 5 commits into from Oct 29, 2019

Conversation

FRidh
Copy link
Member

@FRidh FRidh commented Jun 15, 2019 •
edited

Motivation for this change
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 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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@FRidh
Copy link
Member Author

FRidh commented Jul 11, 2019

cc @NixOS/darwin-maintainers. Should we use Python 3 for the bootstrapping Python or is that orthogonal to this issue?

@FRidh FRidh added the 6.topic: darwin Running or building packages on Darwin label Jul 17, 2019
@FRidh FRidh added this to the 19.09 milestone Sep 8, 2019
@FRidh FRidh mentioned this pull request Sep 10, 2019
Merged
10 tasks
@FRidh FRidh modified the milestones: 19.09, 20.03 Oct 29, 2019
@FRidh
libxml2: use python3 for build and as default for bindings
e685b23 
Changing the default may cause breakage, however, users should have
already switched to `pythonPackages.libxml2` long ago.
@FRidh
libxslt: use python3 for build and as default for bindings
121ede4 
Changing the default may cause breakage, however, users should have already switched to `pythonPackages.libxslt` long ago.
@FRidh FRidh requested a review from jonringer as a code owner October 29, 2019 10:51
@FRidh FRidh added this to Needs review in Python 2 deprecation Oct 29, 2019
@ofborg ofborg bot requested a review from edolstra October 29, 2019 12:22
@FRidh FRidh merged commit 8d92646 into NixOS:staging Oct 29, 2019
@FRidh FRidh deleted the libxml2 branch October 29, 2019 12:47
chkno added a commit to chkno/nixpkgs that referenced this pull request Oct 30, 2019
@chkno
itstool: Don't use Python 3 (yet)
8b3885f 
Per http://itstool.org/download.html , itstool doesn't support python3
until version 2.0.3 (and perhaps doesn't support it correctly until
2.0.5).

This change allows NixOS tests to run again after NixOS#63174 broke
shared-mime-info.
@chkno chkno mentioned this pull request Oct 30, 2019
Closed
10 tasks
@chkno
Copy link
Member

chkno commented Oct 30, 2019

After this change, I get this error:

$ nix-shell -p shared-mime-info -I nixpkgs=.
...
Traceback (most recent call last):
  File "/nix/store/b2cms2q9bmrhp1as93m8vbkjq9r6zn5y-itstool-2.0.2/bin/itstool", line 1545, in <module>
    out = file(opts.output, 'w')
NameError: name 'file' is not defined
make[2]: *** [Makefile:1179: freedesktop.org.xml] Error 1
make[2]: Leaving directory '/build/shared-mime-info-1.13.1'
make[1]: *** [Makefile:731: all-recursive] Error 1
make[1]: Leaving directory '/build/shared-mime-info-1.13.1'
make: *** [Makefile:455: all] Error 2
builder for '/nix/store/40rwsy7q9hbiqhgi5ck5kibmirmywv8k-shared-mime-info-1.13.1.drv' failed with exit code 2
error: build of '/nix/store/40rwsy7q9hbiqhgi5ck5kibmirmywv8k-shared-mime-info-1.13.1.drv' failed

I notice this error when I try to run any NixOS test, such as nix-build nixos/tests/login.nix.

Reverting 8d92646 or makes the error go away, as does dropping itstool back to python2 as in #72335.

itstool says it does not support Python 3 until version 2.0.3, but bumping its version isn't straightforward because itstool/default.nix says 2.0.3+ breaks the build of gnome3.gnome-desktop.

@worldofpeace
Copy link
Contributor

@chkno itstool isn't even used in gnome-desktop anymore, it's likely this issue has been resolved already and we can update itstool, or maybe there's a patch needed in libxml2.

@worldofpeace
Copy link
Contributor

Yeah, distro's have this patch from fedora

to fix the crash. Maybe it has been committed in a newer version.

@jtojnar
Copy link
Contributor

jtojnar commented Oct 30, 2019

@worldofpeace unfortunately gnome-desktop was not the only affected package, the cause is not fixed https://bugzilla.gnome.org/show_bug.cgi?id=789714

@worldofpeace
Copy link
Contributor

@jtojnar https://src.fedoraproject.org/rpms/libxml2/blob/master/f/libxml2-2.9.8-python3-unicode-errors.patch?

@jtojnar
Copy link
Contributor

jtojnar commented Oct 30, 2019

Yes, that seems to have come from https://bugzilla.opensuse.org/show_bug.cgi?id=1065270. It is definitely better but as https://bugzilla.gnome.org/show_bug.cgi?id=789714#c4 says, it is just a hack for broken libxml2. I wonder why is this not considered a critical security vulnerability.

@worldofpeace
Copy link
Contributor

Yes, that seems to have come from https://bugzilla.opensuse.org/show_bug.cgi?id=1065270. It is definitely better but as https://bugzilla.gnome.org/show_bug.cgi?id=789714#c4 says, it is just a hack for broken libxml2. I wonder why is this not considered a critical security vulnerability.

It seems upstream doesn't find it concerning

chkno added a commit to chkno/nixpkgs that referenced this pull request Oct 31, 2019
@chkno
itstool: 2.0.2 -> 2.0.6
0c999c7 
To get python3 support.  NixOS#63174 flipped itstool to python3, but itstool
doesn't support python3 until 2.0.3 (and perhaps does not support it
well until 2.0.5).

Pressing forward instead of rolling back at worldofpeace's suggestion,
who mentions that other distros seem to be able to ship recent versions
of itstool.

Tensions in this space seem two-fold.  One set centers around libxml2
being a low-level C library with sharp edges, manual memory management,
and performance concerns; the python libxml2 wrapper being quite thin
(the most dubious character in this drama); and python's sentiment that
it ought to be quite hard to crash the interpreter casually.  This comes
to a head in https://gitlab.gnome.org/GNOME/libxml2/issues/12 , where a
use-after-free problem in idiomatic-looking python code is declared
working-as-designed.

The other set is around python3 being more UTF-8-aware than libxml2's
python wrapper, such as https://bugzilla.gnome.org/show_bug.cgi?id=789714
and https://src.fedoraproject.org/rpms/libxml2/blob/master/f/libxml2-2.9.8-python3-unicode-errors.patch

itstool is caught in this crossfire merely for being a widely-used
python program that uses XML.
@chkno chkno mentioned this pull request Oct 31, 2019
Merged
10 tasks
@FRidh FRidh moved this from Needs review to Merged in Python 2 deprecation Nov 3, 2019
@bobrik bobrik mentioned this pull request Jan 29, 2021
Merged
10 tasks
bobrik added a commit to bobrik/nixpkgs that referenced this pull request Jan 30, 2021
@bobrik
libxslt: use python3 on darwin
b5e6d56 
See NixOS#63174 where the condition was first introduced.
bobrik added a commit to bobrik/nixpkgs that referenced this pull request Jan 31, 2021
@bobrik
libxslt: use python3 on darwin
0042388 
See NixOS#63174 where the condition was first introduced.
FRidh pushed a commit that referenced this pull request Feb 1, 2021
@bobrik @FRidh
libxslt: use python3 on darwin
86de926 
See #63174 where the condition was first introduced.
dasj19 pushed a commit to dasj19/nixpkgs that referenced this pull request Feb 11, 2021
@bobrik @dasj19
libxslt: use python3 on darwin
9f4e8a4 
See NixOS#63174 where the condition was first introduced.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotlambda dotlambda Awaiting requested review from dotlambda

@lovek323 lovek323 Awaiting requested review from lovek323

@andir andir Awaiting requested review from andir

@jonringer jonringer Awaiting requested review from jonringer

@edolstra edolstra Awaiting requested review from edolstra

Assignees

@LnL7 LnL7

@7c6f434c 7c6f434c

Labels
6.topic: darwin Running or building packages on Darwin 6.topic: python 8.has: package (new) 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
Python 2 deprecation
  
Done
Milestone
20.03
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@FRidh @chkno @worldofpeace @jtojnar @edolstra @LnL7 @7c6f434c