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

ovftool: 4.4.1-16812187 -> 4.5.0-20459872 #214663

Merged
merged 2 commits into from Feb 20, 2023
Merged

ovftool: 4.4.1-16812187 -> 4.5.0-20459872 #214663

merged 2 commits into from Feb 20, 2023

Conversation

numinit
Copy link
Contributor

@numinit numinit commented Feb 5, 2023 •
edited

Use ovftool's bundled OpenSSL since they appear to be tied to it.

On the plus side -- no more FHS env needed.

On the minus side -- no more aarch64 support.

Someone get on VMWare to update ovftool to use a modern version of OpenSSL, seriously.

Description of changes
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
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@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/1795

@numinit
Copy link
Contributor Author

numinit commented Feb 5, 2023 •
edited

To test:

  • NIXPKGS_ALLOW_UNFREE=1 nix build '.#ovftool' --impure
  • ./result/bin/ovftool https://channels.nixos.org/nixos-unstable/latest-nixos-x86_64-linux.ova nixos.ovf

@WolfangAukang
Copy link
Contributor

WolfangAukang commented Feb 5, 2023 •
edited

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

1 package built:
  • ovftool

Tested it with a local OVA that I generally convert to OVF for a certain process. Worked fine.

[nix-shell:~/.cache/nixpkgs-review/pr-214663-1]$ ./results/ovftool/bin/ovftool --overwrite --lax ~/Packer/builds/vmware/test/20230106-001.ov{a,f}
Opening OVA source: /home/albino/Packer/builds/vmware/test/20230106-001.ova
Opening OVF target: /home/albino/Packer/builds/vmware/test/20230106-001.ovf
Writing OVF package: /home/albino/Packer/builds/vmware/test/20230106-001.ovf
Transfer Completed
Warning:
 - No supported manifest(sha1, sha256, sha512) entry found for: '20230106-001-disk001.vmdk'.
 - No manifest file found.
Completed successfully

SuperSandro2000
SuperSandro2000 reviewed Feb 6, 2023
View reviewed changes
pkgs/tools/virtualization/ovftool/default.nix
filename = "VMware-ovftool-${version}-lin.x86_64.zip";
url = "${baseUrl}/f87355ff-f7a9-4532-b312-0be218a92eac/b2916af6-9f4f-4112-adac-49d1d6c81f63/${filename}";
sha256 = "1fkm18yfkkm92m7ccl6b4nxy5lagwwldq56b567091a5sgad38zw";
unpackPhase = ovftoolZipUnpackPhase;
};
};
Copy link
Member

Choose a reason for hiding this comment

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

This needs a or throw unsupported system otherwise eval fails on other platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's actually just below.

@ofborg ofborg bot requested a review from WolfangAukang February 13, 2023 10:38
@numinit numinit mentioned this pull request Feb 14, 2023
Closed
@numinit
Copy link
Contributor Author

numinit commented Feb 15, 2023 •
edited

Added an installCheckPhase that uses a handcrafted OVA of NixOS with a 1 MiB root disk containing all zeroes.

@numinit
Copy link
Contributor Author

numinit commented Feb 15, 2023

Oof, looks like it's trying to editorconfig check the OVA... 🤦

pkgs/tools/virtualization/ovftool/installCheckPhase.ova:
Wrong line endings or new final newline
Not all lines have the correct end of line character
138: Trailing whitespace

@numinit
Copy link
Contributor Author

numinit commented Feb 15, 2023

In case you're wondering what's in the ovf: https://gist.github.com/numinit/7f79b3387cbc83a1111dd5da5ef176d2

@numinit numinit force-pushed the update-ovftool branch 2 times, most recently from 73db240 to 9b3a1a0 Compare February 16, 2023 07:52
@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/1846

@WolfangAukang
Copy link
Contributor

@numinit can you please squash your commits related to the package modification?

SuperSandro2000
SuperSandro2000 reviewed Feb 20, 2023
View reviewed changes
pkgs/tools/virtualization/ovftool/default.nix
cd ovftool-check

$out/bin/ovftool ${./installCheckPhase.ova} nixos.ovf
if [ ! -f nixos.ovf ] || [ ! -f nixos.mf ] || [ ! -f nixos-disk1.vmdk ]; then
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if [ ! -f nixos.ovf ] || [ ! -f nixos.mf ] || [ ! -f nixos-disk1.vmdk ]; then
if [[ ! -f nixos.ovf || ! -f nixos.mf || ! -f nixos-disk1.vmdk ]]; then

pkgs/tools/virtualization/ovftool/default.nix
@@ -116,12 +70,12 @@ stdenv.mkDerivation rec {

nativeBuildInputs = [ autoPatchelfHook makeWrapper unzip ];

preferLocalBuild = true;

sourceRoot = ".";

unpackPhase = ovftoolSystem.unpackPhase;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
unpackPhase = ovftoolSystem.unpackPhase;
inherit (ovftoolSystem) unpackPhase;

@SuperSandro2000 SuperSandro2000 merged commit 92247c1 into NixOS:master Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@WolfangAukang WolfangAukang Awaiting requested review from WolfangAukang

@Mic92 Mic92 Awaiting requested review from Mic92 Mic92 is a code owner

@zowoq zowoq Awaiting requested review from zowoq zowoq is a code owner

Assignees
No one assigned
Labels
8.has: clean-up 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@numinit @nixos-discourse @WolfangAukang @SuperSandro2000