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

inkscape: 1.2.2 -> 1.3, lib2geom: 1.2.2 -> 1.3 #245431

Merged
merged 5 commits into from
Nov 6, 2023
Merged

Conversation

leiserfg
Copy link
Contributor

@leiserfg leiserfg commented Jul 25, 2023

  • inkscape: 1.2.2 -> 1.3, lib2geom: 1.2.2 -> 1.3
Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@leiserfg leiserfg changed the base branch from master to staging July 25, 2023 19:33
@leiserfg
Copy link
Contributor Author

I updated both in a single commit because Inkscape depends on lib2geom (same authors) and is the only dependant of it.

@jtojnar
Copy link
Member

jtojnar commented Jul 25, 2023

Unfortunately this breaks on Darwin https://gitlab.com/inkscape/lib2geom/-/issues/62 and aarch64-linux https://gitlab.com/inkscape/lib2geom/-/issues/63

@leiserfg
Copy link
Contributor Author

Then we will have to wait for a 1.3.1

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/update-inkscape-1-3/31570/2

@vcunat vcunat marked this pull request as draft August 20, 2023 06:29
@eclairevoyant eclairevoyant mentioned this pull request Aug 21, 2023
12 tasks
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/inkscape-possible-domain-compromise/32008/5

@jtojnar
Copy link
Member

jtojnar commented Sep 3, 2023

I updated both in a single commit because Inkscape depends on lib2geom (same authors) and is the only dependant of it.

Separate commits are generally preferred. Then you can link the relevant changelogs in the commit message easily, among other benefits. If you update lib2geom first, Inkscape should still work, unless the library broke the API.

@TheLostLambda
Copy link

Is this still waiting on upstream fixes, or is this close to merging?

Let me know if I can help any!

@leiserfg
Copy link
Contributor Author

Is this still waiting on upstream fixes, or is this close to merging?

Check
https://gitlab.com/inkscape/lib2geom/-/issues/63

@leiserfg
Copy link
Contributor Author

leiserfg commented Sep 12, 2023

@jtojnar can you take over this? I can't test it on Arm anyway.

@jtojnar jtojnar changed the base branch from staging to master September 12, 2023 19:27
@jtojnar jtojnar self-assigned this Sep 12, 2023
@jtojnar jtojnar force-pushed the staging branch 3 times, most recently from 18bdad9 to c4378d5 Compare September 12, 2023 19:40
@ofborg ofborg bot requested a review from jtojnar September 12, 2023 21:46
@jtojnar jtojnar marked this pull request as ready for review September 13, 2023 19:56
Copy link
Member

@jtojnar jtojnar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested some basic operations and the hpgl import. Console complains about resource missing but apply-transform extension still appears to work and either way, it is not regression compared to 1.2:

$ nix-shell -I nixpkgs=$PWD -p "inkscape-with-extensions.override { inkscapeExtensions = null; }" --run inkscape
** (org.inkscape.Inkscape:252608): WARNING **: 22:38:53.378: Failed to find resource file 'inkex.py'. Looked in:
	/home/jtojnar/.config/inkscape/extensions/inkex.py
	(null)
	/nix/store/cyfbhp0rx19gcrjmdka3vryy0bhnhl09-inkscape-with-extensions-1.3/share/inkscape/extensions/inkex.py

** (org.inkscape.Inkscape:252608): WARNING **: 22:38:53.786: You have a huge number of font families (1711), and Cairo is limiting the size of widgets you can draw.
Your preview cell height is capped to 17.

@jtojnar jtojnar removed their assignment Sep 13, 2023
@ofborg ofborg bot requested a review from jtojnar September 13, 2023 21:57
@delroth delroth added 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels Sep 15, 2023
Copy link
Member

@raboof raboof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From https://gitlab.com/inkscape/lib2geom/-/issues/63 I get the impression that we shouldn't let that issue hold up the merge, right?

Tested this build with nix-shell -p "inkscape-with-extensions.override { inkscapeExtensions = [ inkscape-extensions.inkcut ]; }" --run inkscape, creating a simple image and opening it in inkcut. Unfortunately there seems to be something broken with the way inkcut converts objects to paths, but that seems to be an inkscape/inkcut problem, not a reason to hold back this PR IMHO (as inkcut nixpkgs package maintainer).

@raboof raboof mentioned this pull request Oct 4, 2023
10 tasks
@jtojnar jtojnar mentioned this pull request Oct 4, 2023
12 tasks
@jtojnar
Copy link
Member

jtojnar commented Oct 4, 2023

Status of filelock dependency still unclear, see #258885 (comment)

@arkivm
Copy link
Contributor

arkivm commented Oct 4, 2023

v1.3 is still broken on darwin, and v1.3.1 is not going to be a long wait (Nov 15).

leiserfg and others added 4 commits October 26, 2023 13:38
This partly reverts 5d12e40 to avoid potential conflicts.
Also move dependencies only introduced because of tests to checkInputs.
The dependency is not imported anywhere in the Inkscape code so let’s make the requirement explicit.
https://inkscape.org/doc/release_notes/1.3/Inkscape_1.3.html

libepoxy for experimental GPU-accelerated rendering.
pyparsing for HPGL support.

Co-authored-by: Jan Tojnar <jtojnar@gmail.com>
wegank
wegank previously approved these changes Nov 6, 2023
@wegank wegank dismissed their stale review November 6, 2023 03:25

Doesn't really work on darwin...

@wegank
Copy link
Member

wegank commented Nov 6, 2023

@ofborg build inkscape inkscape.passthru.tests

@wegank wegank merged commit 250c07f into NixOS:master Nov 6, 2023
22 of 26 checks passed
@raboof
Copy link
Member

raboof commented Nov 6, 2023

Tested this build with nix-shell -p "inkscape-with-extensions.override { inkscapeExtensions = [ inkscape-extensions.inkcut ]; }" --run inkscape, creating a simple image and opening it in inkcut. Unfortunately there seems to be something broken with the way inkcut converts objects to paths, but that seems to be an inkscape/inkcut problem

(created inkcut/inkcut#374 for this problem)

@raboof
Copy link
Member

raboof commented May 28, 2024

Tested this build with nix-shell -p "inkscape-with-extensions.override { inkscapeExtensions = [ inkscape-extensions.inkcut ]; }" --run inkscape, creating a simple image and opening it in inkcut. Unfortunately there seems to be something broken with the way inkcut converts objects to paths, but that seems to be an inkscape/inkcut problem

(created inkcut/inkcut#374 for this problem)

(not sure what changed, but this works fine now)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: python 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants