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

apriltag: init at v3.4.3 #392308

Merged
merged 2 commits into from
Mar 24, 2025
Merged

apriltag: init at v3.4.3 #392308

merged 2 commits into from
Mar 24, 2025
+52 −0

Conversation

phodina
Copy link

@phodina phodina commented Mar 23, 2025 •
edited
Loading

Add new package for Computer Vision - apriltag.

https://april.eecs.umich.edu/software/apriltag
https://github.com/AprilRobotics/apriltag

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

Other PRs:

@phodina phodina force-pushed the apriltag branch 6 times, most recently from e32b30b to 5e24eb8 Compare March 23, 2025 09:43
SigmaSquadron
SigmaSquadron reviewed Mar 23, 2025
View reviewed changes
Copy link
Contributor

@SigmaSquadron SigmaSquadron left a comment

Choose a reason for hiding this comment

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

Thanks for packaging apriltag. I've left some review comments.

pkgs/by-name/ap/apriltag/package.nix Outdated

buildInputs = [ opencv4WithGtk ];

propagatedBuildInputs = [ python3.pkgs.numpy ];
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be a better idea to wrap the resulting binary instead. Propagating build inputs should be used sparingly, and only when a wrapper isn't a viable solution.

Copy link
Author

Choose a reason for hiding this comment

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

Seems to run correctly even without the numpy and propagatedBuildInputs

@github-actions github-actions bot added 6.topic: vim 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` labels Mar 23, 2025
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 23, 2025
@SigmaSquadron
Copy link
Contributor

@infinisil Shouldn't the OWNERS bot prevent exactly this situation?

@SigmaSquadron
Copy link
Contributor

@phodina: You have rewritten three commits from khaneliman and pushed them here. Please remove them from your branch.

@infinisil
Copy link
Member

@infinisil Shouldn't the OWNERS bot prevent exactly this situation?

It only checks for exact commits from the development branches being included in PRs, I don't think that's the case here.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 23, 2025
@phodina
Copy link
Author

phodina commented Mar 23, 2025

Removed the additional commits that were picked in rebase.

@phodina phodina requested a review from SigmaSquadron March 23, 2025 17:13
@mrcjkb mrcjkb removed their request for review March 23, 2025 19:24
@phodina phodina requested a review from GaetanLepage March 24, 2025 08:16
@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 392308

x86_64-linux

✅ 1 package built:
  • apriltag

aarch64-linux

✅ 1 package built:
  • apriltag

x86_64-darwin

✅ 1 package built:
  • apriltag

aarch64-darwin

✅ 1 package built:
  • apriltag

GaetanLepage
GaetanLepage approved these changes Mar 24, 2025
View reviewed changes
Copy link
Contributor

@GaetanLepage GaetanLepage left a comment

Choose a reason for hiding this comment

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

LGTM

@GaetanLepage GaetanLepage merged commit f9df9fc into NixOS:master Mar 24, 2025
26 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GaetanLepage GaetanLepage

@PerchunPak PerchunPak

@khaneliman khaneliman

@SigmaSquadron SigmaSquadron

Assignees
No one assigned
Labels
8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@phodina @SigmaSquadron @infinisil @GaetanLepage