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

ppsspp: 1.1.0 → 1.3 #23970

Closed
wants to merge 0 commits into from
Closed

ppsspp: 1.1.0 → 1.3 #23970

wants to merge 0 commits into from

Conversation

therealpxc
Copy link
Contributor

Motivation for this change

new upstream release

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@therealpxc, thanks for your PR! By analyzing the history of the files in this pull request, we identified @AndersonTorres, @abbradar and @Fuuzetsu to be potential reviewers.

ndowens
ndowens suggested changes Mar 17, 2017
View reviewed changes
pkgs/misc/emulators/ppsspp/default.nix Outdated
@@ -14,9 +14,9 @@ stdenv.mkDerivation rec{

src = fetchgit {
Copy link
Contributor

Choose a reason for hiding this comment

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

fetchFromGitHub is preferred over fetchgit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think PPSSPP needs to use fetchgit because the repo uses submodules and fetchFromGitHub currently lacks submodule support.

pkgs/misc/emulators/ppsspp/default.nix Outdated
@@ -14,9 +14,9 @@ stdenv.mkDerivation rec{

src = fetchgit {
url = "https://github.com/hrydgard/ppsspp.git";
rev = "8c8e5de89d52b8bcb968227d96cbf049d04d1241";
rev = "6d0d36bf914a3f5373627a362d65facdcfbbfe5f";
Copy link
Contributor

Choose a reason for hiding this comment

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

You can do rev = "v${version}"; here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like I have to explicitly put "refs/tags/v$version". Is that expected? Has that changed recently?

pkgs/misc/emulators/ppsspp/default.nix Outdated
fetchSubmodules = true;
sha256 = "1q21qskzni0nvz2yi2m17gjh4i9nrs2l4fm4y2dww9m29xpvzw3x";
sha256 = "0l8vgdlw657r8gv7rz8iqa6zd9zrbzw10pwhcnahzil7w9qrd03g";
};

buildInputs = [ zlib libpng pkgconfig qt4 qmake4Hook ]
Copy link
Contributor

Choose a reason for hiding this comment

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

pkgconfig qmake4Hook belongs in nativeBuildInputs

matthiasbeyer
matthiasbeyer requested changes Mar 17, 2017
View reviewed changes
Copy link
Contributor

@matthiasbeyer matthiasbeyer left a comment

Choose a reason for hiding this comment

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

Is this a unicode character in the commit message? If yes, please use the normal "->" ascii characters.

@pSub pSub added the 8.has: package (update) This PR updates a package to a newer version label Mar 17, 2017
@ndowens ndowens force-pushed the ppsspp branch 2 times, most recently from 442c5e6 to feefbf7 Compare March 23, 2017 21:17
@ndowens ndowens closed this Mar 23, 2017
@ndowens
Copy link
Contributor

ndowens commented Mar 23, 2017 •
edited
Loading

Sorry, something happened with pushing changes to this; I was told it may have been because used pull instead of fetch. Reopen if you still want to submit it

I couldn't get it to build correctly either, at first it was missing ffmpeg for dep and then it gave a can't find a file that the program should have provided it seems

@therealpxc
Copy link
Contributor Author

therealpxc commented Mar 24, 2017 •
edited
Loading

I think both of those build issues may be caused by the switch to fetchFromGitHub, if you did that in your version, because it would then have failed to include the required submodules.

I've been running this version on NixOS 16.09 for some time, so I know it works there. I've made the recommended changes and I'm rebuilding on my system now to verify that it builds on this branch as well and not just the stable channel.

I'll force push and resubmit for your review once I can verify that the build works. No worries on clobbering it; thankfully Git is good. Thanks for your time and oversight. :-)

@therealpxc therealpxc mentioned this pull request Mar 24, 2017
Merged
7 tasks
@ndowens
Copy link
Contributor

ndowens commented Mar 24, 2017

Yeah it might have been the switch. Don't know why. Sorry again about the other PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthiasbeyer matthiasbeyer matthiasbeyer requested changes

@ndowens ndowens ndowens requested changes

Assignees
No one assigned
Labels
8.has: package (update) This PR updates a package to a newer version
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@therealpxc @mention-bot @ndowens @matthiasbeyer @pSub