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

ssm-session-manager-plugin: add "aarch64-linux" support #183260

Merged
merged 1 commit into from
Aug 2, 2022

Conversation

WhyNotHugo
Copy link
Contributor

@WhyNotHugo WhyNotHugo commented Jul 28, 2022

Description of changes

This package has an if that covers only linux on amd64 and mac on all platforms. This adds support for linux of arm64.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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 I am not using nixOS, so cannot run this.
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

I created a flake that includes this package and it build and worked fine.

CC: @mbaillie

@WhyNotHugo WhyNotHugo changed the title Add aarch64-linux for ssm-session-manager-plugin ssm-session-manager-plugin: add "aarch64-linux" support Jul 28, 2022
@WhyNotHugo
Copy link
Contributor Author

WhyNotHugo commented Jul 28, 2022

@martinbaillie I think you are the correct maintainer and the file has the wrong username?

@WhyNotHugo
Copy link
Contributor Author

@maxeaubrey Maybe you're the right person to ping? :)

@amaxine
Copy link
Contributor

amaxine commented Jul 31, 2022

@WhyNotHugo looks good but I don't have an arm64 nix box to verify. I'm also not sure of the "correctness" of the way the different archs are gated (just visually comparing with other derivations in nixpkgs, not that there's not 50 different approaches 😅) - I'd suggest posting in the "PRs ready for review" thread.

@WhyNotHugo
Copy link
Contributor Author

This one? Done.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1053

@WhyNotHugo WhyNotHugo force-pushed the ssm-aarch64 branch 4 times, most recently from 1c7a2dc to 65b9fbf Compare August 2, 2022 13:48
@WhyNotHugo
Copy link
Contributor Author

CI seems happy now. I've tested building on aarch64 and amd64.

@SuperSandro2000
Copy link
Member

@ofborg build ssm-session-manager-plugin

@SuperSandro2000
Copy link
Member

Can you please fix the commit message to fit the contributing guide? Then ofborg would also be auto triggered.

After that we can merge this.

@WhyNotHugo
Copy link
Contributor Author

Can you please fix the commit message to fit the contributing guide?

https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#writing-good-commit-messages

I don't see any specific convention mentioned. History makes it obvious what I should use, but perhaps an explicit mention in the docs would help? I'm very sure my previous contributions don't fit this very clear convention.

@WhyNotHugo
Copy link
Contributor Author

WhyNotHugo commented Aug 2, 2022

I think ofborg is happy now.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 merged commit 731428c into NixOS:master Aug 2, 2022
@WhyNotHugo WhyNotHugo deleted the ssm-aarch64 branch August 4, 2022 09:43
@WhyNotHugo
Copy link
Contributor Author

Thanks!

1 similar comment
@WhyNotHugo
Copy link
Contributor Author

WhyNotHugo commented Oct 11, 2022 via email

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.

None yet

5 participants