Skip to content

Comments

vpkedit: init at 4.4.2#391455

Merged
GaetanLepage merged 2 commits intoNixOS:masterfrom
RdrSeraphim:add-vpkedit
Apr 2, 2025
Merged

vpkedit: init at 4.4.2#391455
GaetanLepage merged 2 commits intoNixOS:masterfrom
RdrSeraphim:add-vpkedit

Conversation

@RdrSeraphim
Copy link
Member

Adds VPKEdit, a tool to {create,peruse,modify} pack file formats of various game engines.

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.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Mar 20, 2025
@RdrSeraphim
Copy link
Member Author

Thank you!

@0xda157
Copy link
Contributor

0xda157 commented Mar 23, 2025

Could you squash your commits into one please.

@RdrSeraphim RdrSeraphim requested a review from 0xda157 March 23, 2025 22:09
Copy link
Contributor

@0xda157 0xda157 left a comment

Choose a reason for hiding this comment

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

I'm not very familiar with dpkg so someone else should do the rest of the reviewing on this.

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.

The produced output does not have a bin/ directory. You may have to investigate some more.

@RdrSeraphim
Copy link
Member Author

The produced output does not have a bin/ directory. You may have to investigate some more.

I've been able to get this going, although with all the Qt dependency the result is some subfolders in bin/ for Qt plugins. I'm not sure if that's acceptable for the ideal package structure but this is the only way the program runs, it seems. Should I look into a different solution or is this fine?

@GaetanLepage
Copy link
Contributor

GaetanLepage commented Mar 31, 2025

Well, we need to have at least the program's main executable in $out/bin. Having non-executable dependencies in $out/bin is also not ideal.

@RdrSeraphim
Copy link
Member Author

This leaves this with the build-from-source option then. I'll take a look into their build workflows and see how they piece it together.

@RdrSeraphim
Copy link
Member Author

RdrSeraphim commented Apr 1, 2025

I just discovered that #326999 exists, which builds from source. It's behind a couple minor versions but I've made some adjustments to handle the vendored dependencies without cmake trying to fetch repos on its own.

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.

Nice if we can build it from source!
Maybe consider adding

Co-authored-by: HurricanePootis <53066639+HurricanePootis@users.noreply.github.com>

In the commit description to give credits to @HurricanePootis

@RdrSeraphim
Copy link
Member Author

Update now stores translation data in lib/vpkedit/i18n and uses {$XDG_CONFIG_HOME,$HOME/.config}/vpkedit/config.ini for the configuration path. Executables now in /bin properly too. Thanks :)

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 391455


x86_64-linux

✅ 1 package built:
  • vpkedit

@RdrSeraphim RdrSeraphim requested a review from 0xda157 April 1, 2025 21:20
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.

This looks very good!
Can you add a comment detailing how the various *-src should be updated.
For instance a link to the upstream file that references the needed commit hashes.

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 391455


x86_64-linux

✅ 1 package built:
  • vpkedit

Co-authored-by: HurricanePootis <53066639+HurricanePootis@users.noreply.github.com>
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.

All good! Thank you for your patience @SeraphimRP !
I have just force-pushed minor changes so that we could avoid yet another review round.

Good job :)

@GaetanLepage GaetanLepage merged commit 42e33dd into NixOS:master Apr 2, 2025
27 of 29 checks passed
@RdrSeraphim RdrSeraphim deleted the add-vpkedit branch April 2, 2025 13:05
@RdrSeraphim RdrSeraphim mentioned this pull request Apr 2, 2025
13 tasks
@craftablescience craftablescience mentioned this pull request Apr 4, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants