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

zchunk: 1.1.6-> 1.1.8 & Add darwin support #107443

Merged
merged 1 commit into from Dec 29, 2020
Merged

zchunk: 1.1.6-> 1.1.8 & Add darwin support #107443

merged 1 commit into from Dec 29, 2020

Conversation

nima2007
Copy link
Contributor

@nima2007 nima2007 commented Dec 23, 2020 •
edited

Motivation for this change

Zchunk previously did not compile on macOS, so it had to be disabled in librepo for macOS (#107104).
With this change, Zchunk will compile on macOS and we will be able to enable Zchunk for librepo.

Things done

-Updated version and hash from 1.1.6 to 1.1.8
-Added a patch to use the macOS location of endian if compiling on macOS
-Added argp-standalone as a build input for macOS
-Added a patch to find the arg library if compiling on macOS

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS via nix-review bc I didn't know how to do it any other way
    • macOS On macOS Catalina and Big Sur via nix-build & nix-review. https://pastebin.com/tjxtikkn
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests) Did not find any related tests
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip" Ran nixpkgs-review pr successfully but I didn't use that exact command
  • Tested execution of all binary files (usually in ./result/bin/) https://pastebin.com/RDQwfGFV
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date No documentation
  • Fits CONTRIBUTING.md. Squashed to a single commit following commit message format

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Dec 23, 2020
rb2k
rb2k reviewed Dec 23, 2020
View reviewed changes
pkgs/development/libraries/zchunk/darwin.patch Outdated
@@ -0,0 +1,26 @@
diff --git a/meson.build b/meson.build
Copy link
Contributor

Choose a reason for hiding this comment

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

zchunk/zchunk#24 mentions that in theory they have osx. I wonder if we can contribute that.

(would still be required here until they merge)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am seeing both the endian and argp problems mentioned in that issue, but I don't see a fix in any commits. It looks like someone figured it out without making a pull request. So yes, it would make sense for me to make a pull request there too, and then eventually remove the patch here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that's a blocker, but I think that would be smart.
Otherwise we'll just have to adapt the patch for future updates when it would be easier to just have it done upstream.

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 107443 run on x86_64-darwin 1

1 package built:
  • zchunk

@nima2007 nima2007 marked this pull request as ready for review December 23, 2020 08:50
@nima2007
Copy link
Contributor Author

Result of nixpkgs-review pr 107443 1

8 packages built:
  • libdnf
  • librepo
  • microdnf
  • python37Packages.librepo
  • python38Packages.librepo
  • python39Packages.librepo
  • rpm-ostree
  • zchunk

@rb2k
Copy link
Contributor

rb2k commented Dec 23, 2020

Result of nixpkgs-review pr 107443 run on x86_64-darwin 1

1 package built:
  • zchunk

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 107443 1

8 packages built:

please update your nixpkgs-review.

@rb2k
Copy link
Contributor

rb2k commented Dec 23, 2020

Result of nixpkgs-review pr 107443 run on x86_64-linux 1

8 packages built:
  • libdnf
  • librepo
  • microdnf
  • python37Packages.librepo
  • python38Packages.librepo
  • python39Packages.librepo
  • rpm-ostree
  • zchunk

@nima2007
zchunk: 1.1.6 -> 1.1.8 | Add darwin support
a847120 
Zchunk previously couldn't compile on macOS and had to be skipped for macOS on some packages.
@grahamc grahamc merged commit 09b571c into NixOS:master Dec 29, 2020
@nima2007 nima2007 mentioned this pull request Dec 30, 2020
Closed
8 tasks
This was referenced Dec 30, 2020
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rb2k rb2k rb2k left review comments

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 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
5 participants
@nima2007 @SuperSandro2000 @rb2k @grahamc @Janik-Haag