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.

Sign up for GitHub

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

xmlcopyeditor: init at 1.2.1.3 #38545

Merged
merged 3 commits into from Nov 18, 2018
Merged

xmlcopyeditor: init at 1.2.1.3 #38545

merged 3 commits into from Nov 18, 2018

Conversation

candeira
Copy link
Contributor

@candeira candeira commented Apr 7, 2018

Motivation for this change

I use xmlcopyeditor, and it's not present in nixpkgs

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
  • Fits CONTRIBUTING.md.

infinisil
infinisil requested changes Apr 8, 2018
View reviewed changes
Copy link
Member

@infinisil infinisil left a comment •
edited

Choose a reason for hiding this comment

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

When I run it I get the error

SSE2 is enabled in Xerces-C++ library. Xerces-C++ didn't use them in a thread-safe way. It may cause program crashes (segmentation faults).

If it happens, please try compiling Xerces-C++ with SSE2 disabled.

Can you try do that? This would involve adding support for disabling this SSE2 thing in xerces and usint xercesc.override { enableSSE2 = false; } in your build inputs instead.

Other than this error the editor is working just fine, nice first PR!

pkgs/applications/editors/xmlcopyeditor/default.nix Outdated Show resolved Hide resolved
pkgs/applications/editors/xmlcopyeditor/default.nix Outdated Show resolved Hide resolved
pkgs/applications/editors/xmlcopyeditor/default.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
matthewbauer
matthewbauer requested changes Apr 25, 2018
View reviewed changes
Copy link
Member

@matthewbauer matthewbauer left a comment

Choose a reason for hiding this comment

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

See @infinisil's review

@jtojnar
Copy link
Contributor

jtojnar commented Nov 11, 2018

Any progress on this?

@c0bw3b c0bw3b mentioned this pull request Nov 18, 2018
Merged
9 tasks
jtojnar
jtojnar reviewed Nov 18, 2018
View reviewed changes
pkgs/applications/editors/xmlcopyeditor/xmlcopyeditor.patch
$(srcdir)/xmlschemaparser.cpp

-AM_CPPFLAGS = -I/usr/include/libxml2 $(ENCHANT_CFLAGS) $(GTK_CFLAGS)
+AM_CPPFLAGS = -I$(CPLUS_INCLUDE_PATH) $(ENCHANT_CFLAGS) $(GTK_CFLAGS)
Copy link
Contributor

Choose a reason for hiding this comment

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

Opened an issue about this: https://sourceforge.net/p/xml-copy-editor/bugs/222/

@c0bw3b
Copy link
Contributor

c0bw3b commented Nov 18, 2018

@GrahamcOfBorg build xmlcopyeditor

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: xmlcopyeditor

Partial log (click to expand)

make[2]: Leaving directory '/build/xmlcopyeditor-1.2.1.3'
make[1]: Leaving directory '/build/xmlcopyeditor-1.2.1.3'
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/bxiimickdp7ff1cjqapilax1c8dv5gx1-xmlcopyeditor-1.2.1.3
shrinking /nix/store/bxiimickdp7ff1cjqapilax1c8dv5gx1-xmlcopyeditor-1.2.1.3/bin/xmlcopyeditor
gzipping man pages under /nix/store/bxiimickdp7ff1cjqapilax1c8dv5gx1-xmlcopyeditor-1.2.1.3/share/man/
strip is /nix/store/vcc4svb8gy29g4pam2zja6llkbcwsyiq-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/bxiimickdp7ff1cjqapilax1c8dv5gx1-xmlcopyeditor-1.2.1.3/bin
patching script interpreter paths in /nix/store/bxiimickdp7ff1cjqapilax1c8dv5gx1-xmlcopyeditor-1.2.1.3
checking for references to /build in /nix/store/bxiimickdp7ff1cjqapilax1c8dv5gx1-xmlcopyeditor-1.2.1.3...

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: xmlcopyeditor

Partial log (click to expand)

make[1]: Leaving directory '/build/xmlcopyeditor-1.2.1.3'
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/r29bv60x790yq76yaqsqayygbl3c6r57-xmlcopyeditor-1.2.1.3
shrinking /nix/store/r29bv60x790yq76yaqsqayygbl3c6r57-xmlcopyeditor-1.2.1.3/bin/xmlcopyeditor
gzipping man pages under /nix/store/r29bv60x790yq76yaqsqayygbl3c6r57-xmlcopyeditor-1.2.1.3/share/man/
strip is /nix/store/p9akxn2sfy4wkhqdqa3li97pc6jaz3r1-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/r29bv60x790yq76yaqsqayygbl3c6r57-xmlcopyeditor-1.2.1.3/bin
patching script interpreter paths in /nix/store/r29bv60x790yq76yaqsqayygbl3c6r57-xmlcopyeditor-1.2.1.3
checking for references to /build in /nix/store/r29bv60x790yq76yaqsqayygbl3c6r57-xmlcopyeditor-1.2.1.3...
/nix/store/r29bv60x790yq76yaqsqayygbl3c6r57-xmlcopyeditor-1.2.1.3

jtojnar
jtojnar reviewed Nov 18, 2018
View reviewed changes
pkgs/applications/editors/xmlcopyeditor/default.nix

stdenv.mkDerivation rec {
name = "xmlcopyeditor-${version}";
version = "1.2.1.3";
Copy link
Contributor

Choose a reason for hiding this comment

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

1.2.1.5 is available

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a WIP macOS (only?) port as I understand it.

http://xml-copy-editor.sourceforge.net/ and https://sourceforge.net/projects/xml-copy-editor/files/xmlcopyeditor-linux/ still points to v1.2.1.3 for Linux (or Windows).

Copy link
Contributor

Choose a reason for hiding this comment

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

Websites are often outdated, I would follow the changelog: https://sourceforge.net/p/xml-copy-editor/code/ci/master/tree/ChangeLog

Copy link
Contributor

Choose a reason for hiding this comment

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

The second link is not the front website but the list of source archives for Linux.
https://repology.org/metapackage/xmlcopyeditor/versions
No one ships v1.2.1.5.

So I'm going to stick to the version intended by the original author of this PR who's going to be its maintainer. @candeira should open a new PR later once upstream got its release process straighten out.

@c0bw3b
Copy link
Contributor

c0bw3b commented Nov 18, 2018

Also no more warning about Xerces-C being built with SSE2 at first launch after #50536 (tested locally)

@c0bw3b c0bw3b merged commit 2c08fe5 into NixOS:master Nov 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtojnar jtojnar jtojnar left review comments

@infinisil infinisil infinisil requested changes

@c0bw3b c0bw3b c0bw3b left review comments

@matthewbauer matthewbauer matthewbauer requested changes

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@candeira @jtojnar @c0bw3b @GrahamcOfBorg @matthewbauer @infinisil @Janik-Haag