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

ocamlPackages.ezxmlm: init at 1.0.2 #47904

Merged
merged 1 commit into from Oct 6, 2018
Merged

ocamlPackages.ezxmlm: init at 1.0.2 #47904

merged 1 commit into from Oct 6, 2018

Conversation

carlosdagos
Copy link
Member

@carlosdagos carlosdagos commented Oct 5, 2018 •
edited

Motivation for this change

Nice library to have :)

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.

@srhb
Copy link
Contributor

srhb commented Oct 5, 2018

@GrahamcOfBorg build ocamlPackages.ezxmlm

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: ocamlPackages.ezxmlm

Partial log (click to expand)

Processing file ezxmlm.install as ezxmlm.
post-installation fixup
moving /nix/store/ccqnqs0zskhvc2kz0pyzx781z3dmshlh-ocaml4.06.1-ezxmlm-1.0.2/doc to /nix/store/ccqnqs0zskhvc2kz0pyzx781z3dmshlh-ocaml4.06.1-ezxmlm-1.0.2/share/doc
shrinking RPATHs of ELF executables and libraries in /nix/store/ccqnqs0zskhvc2kz0pyzx781z3dmshlh-ocaml4.06.1-ezxmlm-1.0.2
shrinking /nix/store/ccqnqs0zskhvc2kz0pyzx781z3dmshlh-ocaml4.06.1-ezxmlm-1.0.2/lib/ocaml/4.06.1/site-lib/ezxmlm/ezxmlm.cmxs
strip is /nix/store/428gs2z4b8f9byvghzlpbjwjb3a7jwxx-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/ccqnqs0zskhvc2kz0pyzx781z3dmshlh-ocaml4.06.1-ezxmlm-1.0.2/lib
patching script interpreter paths in /nix/store/ccqnqs0zskhvc2kz0pyzx781z3dmshlh-ocaml4.06.1-ezxmlm-1.0.2
checking for references to /build in /nix/store/ccqnqs0zskhvc2kz0pyzx781z3dmshlh-ocaml4.06.1-ezxmlm-1.0.2...
/nix/store/ccqnqs0zskhvc2kz0pyzx781z3dmshlh-ocaml4.06.1-ezxmlm-1.0.2

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: ocamlPackages.ezxmlm

Partial log (click to expand)

building
jbuilder build @install --dev
installing
Processing file ezxmlm.install as ezxmlm.
post-installation fixup
moving /nix/store/hlwgwyqq0gr43za96blrll4wiphjwljh-ocaml4.06.1-ezxmlm-1.0.2/doc to /nix/store/hlwgwyqq0gr43za96blrll4wiphjwljh-ocaml4.06.1-ezxmlm-1.0.2/share/doc
strip is /nix/store/9xjkb4xz0b5lmizij9ppxy7lkxdxhx6b-cctools-binutils-darwin/bin/strip
stripping (with command strip and flags -S) in /nix/store/hlwgwyqq0gr43za96blrll4wiphjwljh-ocaml4.06.1-ezxmlm-1.0.2/lib
patching script interpreter paths in /nix/store/hlwgwyqq0gr43za96blrll4wiphjwljh-ocaml4.06.1-ezxmlm-1.0.2
/nix/store/hlwgwyqq0gr43za96blrll4wiphjwljh-ocaml4.06.1-ezxmlm-1.0.2

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: ocamlPackages.ezxmlm

Partial log (click to expand)

Processing file ezxmlm.install as ezxmlm.
post-installation fixup
moving /nix/store/8kf28pchkjbygj1p53q25fzn6f5i852p-ocaml4.06.1-ezxmlm-1.0.2/doc to /nix/store/8kf28pchkjbygj1p53q25fzn6f5i852p-ocaml4.06.1-ezxmlm-1.0.2/share/doc
shrinking RPATHs of ELF executables and libraries in /nix/store/8kf28pchkjbygj1p53q25fzn6f5i852p-ocaml4.06.1-ezxmlm-1.0.2
shrinking /nix/store/8kf28pchkjbygj1p53q25fzn6f5i852p-ocaml4.06.1-ezxmlm-1.0.2/lib/ocaml/4.06.1/site-lib/ezxmlm/ezxmlm.cmxs
strip is /nix/store/dxf1m7dhc4qb655bdljc1fsd74v1nag3-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/8kf28pchkjbygj1p53q25fzn6f5i852p-ocaml4.06.1-ezxmlm-1.0.2/lib
patching script interpreter paths in /nix/store/8kf28pchkjbygj1p53q25fzn6f5i852p-ocaml4.06.1-ezxmlm-1.0.2
checking for references to /build in /nix/store/8kf28pchkjbygj1p53q25fzn6f5i852p-ocaml4.06.1-ezxmlm-1.0.2...
/nix/store/8kf28pchkjbygj1p53q25fzn6f5i852p-ocaml4.06.1-ezxmlm-1.0.2

@srhb
Copy link
Contributor

srhb commented Oct 5, 2018

Are you sure this is the MIT license? It looks more like a (mangled...) version of the OpenBSD license to me.

@carlosdagos
Copy link
Member Author

@srhb well spotted. I’m actually not sure 😅 I now remember I needed to look that up. The license file in the repo is a bit weird indeed. Do you reckon we can not include that metadata or is there some default we can use like “unknown”? Thanks for reviewing

@srhb
Copy link
Contributor

srhb commented Oct 5, 2018

I think it's only the formatting that's mangled, but perhaps you can confirm with a proofread: https://en.wikipedia.org/wiki/ISC_license#OpenBSD_license

If that is indeed the case, I think we should just add the openbsd license and refer to that.

@carlosdagos carlosdagos force-pushed the ocaml-ezxmlm branch 2 times, most recently from 7b82c8f to 3de7802 Compare October 5, 2018 12:38
@carlosdagos
Copy link
Member Author

Updated in the latest commit :)

vbgl
vbgl reviewed Oct 5, 2018
View reviewed changes
pkgs/development/ocaml-modules/ezxmlm/default.nix Outdated

buildInputs = [ ocaml findlib dune ];

buildPhase = "make build";
Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to use buildFlags instead. This makes overriding easier, etc.

pkgs/development/ocaml-modules/ezxmlm/default.nix
# The license in the source does not explicitly say isc or openbsd, but
# the text matches it closely:
# https://en.wikipedia.org/wiki/ISC_license#OpenBSD_license
license = licenses.isc;
Copy link
Contributor

Choose a reason for hiding this comment

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

They claim it there: https://github.com/ocaml/opam-repository/blob/master/packages/ezxmlm/ezxmlm.1.0.2/opam#L4 I think you can remove the comment.

pkgs/development/ocaml-modules/ezxmlm/default.nix
repo = "ezxmlm";
rev = "v${version}";
sha256 = "1dgr61f0hymywikn67inq908x5adrzl3fjx3v14l9k46x7kkacl9";
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the release artifact (https://github.com/avsm/ezxmlm/releases/download/v1.0.2/ezxmlm-1.0.2.tbz) is a better source.

Copy link
Member Author

Choose a reason for hiding this comment

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

Tried using this and it requires topkg-jbuilder, which I don't think is currently available in nixpkgs. Don't know if I'm missing something though :)

@carlosdagos
Copy link
Member Author

Thanks for reviewing @vbgl. See latest commit with the changes you requested. Only thing what I couldn't get to work is using the release artifact, so still using fetchFromGithub.

@vbgl vbgl merged commit 364d477 into NixOS:master Oct 6, 2018
@carlosdagos carlosdagos deleted the ocaml-ezxmlm branch October 7, 2018 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vbgl vbgl vbgl left review comments

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@carlosdagos @srhb @GrahamcOfBorg @vbgl