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

immich-go: init at 0.13.2 #303799

Merged
merged 2 commits into from
Apr 17, 2024
Merged

immich-go: init at 0.13.2 #303799

merged 2 commits into from
Apr 17, 2024

Conversation

kai-tub
Copy link
Contributor

@kai-tub kai-tub commented Apr 13, 2024

Description of changes

  • Add immich-go to nixpkgs at the latest release version 0.13.2.
  • Add me as maintainer in the maintainer's list and as a maintainer for the package

Immich-go is a cross-platform Go program that allows bulk-uploading pictures/videos to immich.
It supports importing data from Google photo takeout archives and is recommended/endorsed by the official immich documentation.
But, it is not limited to Google Photos and can iterate over directory trees and zip archives as well.

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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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.

I have one Go-specific nixpkgs question. What is 'official' stance on these linked variables in ldflags, like "-X main.commit=${version}" ?
As a user, I find it a bit confusing if the output of the commit value refers to the version and if the date refers to either a string or UNIX timestamp 0. I have seen that the pdfcpu package is forced to fix the output as upstream checks for these values, but it doesn't seem that this option is generally preferred within nixpkgs.

# Apparently upstream requires that the compiled executable will know the
# commit hash and the date of the commit. This information is also presented
# in the output of `pdfcpu version` which we use as a sanity check in the
# installCheckPhase. This was discussed upstream in:
#
# - https://github.com/pdfcpu/pdfcpu/issues/751
# - https://github.com/pdfcpu/pdfcpu/pull/752
#
# The trick used here is to write that information into files in `src`'s
# `$out`, and then read them into the `ldflags`. We also delete the `.git`
# directories in `src`'s $out afterwards, imitating what's done if
# `leaveDotGit = false;` See also:
# https://github.com/NixOS/nixpkgs/issues/8567
leaveDotGit = true;
postFetch = ''
cd "$out"
git rev-parse HEAD > $out/COMMIT
git log -1 --pretty=%cd --date=format:'%Y-%m-%dT%H:%M:%SZ' > $out/SOURCE_DATE
find "$out" -name .git -print0 | xargs -0 rm -rf
'';
};

So would it be bad style if I apply the same patching logic as used in pdfcpu just because I personally find it more "accessible"?

PS: It is my first time contributing a package. I have tried reading all the relevant documentation, but I could've easily missed something. Let me know how I can improve it!


Add a 👍 reaction to pull requests you find important.

@kirillrdy
Copy link
Member

sorry didn't answer the question in the description

I have one Go-specific nixpkgs question

I think what you did is adequate, I don't think there is one rule that works for everything eg pdfcpu has that constraint.

You could specify rev, and use rev in -X main.commit , at this stage I would say it's up to maintainer eg yourself

@kirillrdy kirillrdy merged commit d52b358 into NixOS:master Apr 17, 2024
26 checks passed
@kai-tub kai-tub deleted the pkgs/immich-go/init branch April 17, 2024 09:35
@kai-tub kai-tub mentioned this pull request Apr 21, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants