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

Alex4BeOS new recipe #631

Closed
wants to merge 1 commit into from
Closed

Alex4BeOS new recipe #631

wants to merge 1 commit into from

Conversation

@Begasus
Copy link
Contributor

Begasus commented Jun 10, 2016

For some reason the app remains in the Deskbar, I need to close it down with PC or when started from Terminal with Ctrl-C ...

@fbrosson
Copy link
Member

fbrosson commented Jun 10, 2016

Since the game has separate archives for the game engine and the data, I think it would be better, IMHO, to create one recipe for the game engine, and another for the game data. There are a few games that have been packaged this way. Here are the recipes for their data part, in case you would like to try splitting Alex4BeOS:

In case any of the links above become outdated, here are the links with today's HEAD revision:

Regarding the header of the recipe, there are some lines that do not follow the recommended order, which is here: https://github.com/haikuports/haikuports/wiki/HaikuPorter-Guidelines#ordering

@Begasus Begasus force-pushed the Begasus:alex4 branch from f623f03 to 13d9222 Jun 11, 2016
@Begasus
Copy link
Contributor Author

Begasus commented Jun 11, 2016

In this case the content of both archives are needed to build the port, one has the original source the other contains BeOS specific files (for instance the Makefile, rsrc and the diff file), so I thought it needed to be together for this one ...
Ordering has been fixed

@fbrosson
Copy link
Member

fbrosson commented Jun 16, 2016

I've built your recipe and it works fine. But I'd like to suggest a few changes and some polishing.
master...fbrosson:alex
I've compared the hpkg I got with my recipe against the one I got with yours, and the result is that they only differ in the .PackageInfo because of a few changes in the SOURCE_URI and of an extra dependency (lib:libaldmb_0.9.3 for REQUIRES and devel:libaldmb for BUILD_REQUIRES).
We don't get any warning without that extra dependency because lib:libdumb_0.9.3 and devel:libdumb come from the same packages as lib:libdumb_0.9.3 and devel:libdumb, but if I'm not mistaken it is better to have these dependencies explicitely.

To grab an exact copy of my polished version of the recipe, remember that you can click on "View" and then chose save on the "Raw" link.
You don't need to grab the patch file because it is identical to yours.
(Off course, you can also grab my commit using git, but for a single file it is eaier to download it with curl or wget or with a browser.)
BTW, I tested the game on x86 but haven't done so on x86_64. If it also works on x86_64 then you might wish to drop the "?" for x86_64 in ARCHITECTURES. I'll let you know as soon as I have built and tested it on x86_64 ;-) Update: It also builds and runs OK on x86_64. Alex does not produce sound on my box, however, even on x86{,_gcc2}, althouth I do have sound working fine with other apps.

@fbrosson
Copy link
Member

fbrosson commented Jun 16, 2016

For some reason the app remains in the Deskbar, I need to close it down with PC or when started from Terminal with Ctrl-C ...

Same thing for me. (but I used the "Team monitor" to kill it.)

Oops, I forgot to remove the empty line between PROVIDES and REQUIRES. (It's fixed now).

@fbrosson
Copy link
Member

fbrosson commented Jun 17, 2016

In this case the content of both archives are needed to build the port, one has the original source the other contains BeOS specific files (for instance the Makefile, rsrc and the diff file), so I thought it needed to be together for this one ..

@Begasus, I forgot to write that you are perfectly right about that ;-)
BTW, the changes I'm suggesting in master...fbrosson:alex are inline with the fact that both archives need to be used in a unique recipe.
The main difference between your recipe and mine is that I have swapped SOURCE_URI and SOURCE_URI_2 because IMHO it is slightly easier to use the original archive as being SOURCE_URI and grab the BeOS/Haiku patch from a SOURCE_URI_2 archive. Doing so allows to simplify the BUILD() function.
The resulting hpkgs, as I explained earlier, are bit-to-bit identical, if you compare the files in your hpkg with the ones in mine, with the single exception of a couple of lines of meta-data in .PackageInfo, of course.
So I hope I have convinced you that the reviewed recipe is also OK ;-)
(Oops, I forgot to test it on x86_64! I'll will do so asap.)
Hmm, we do not use the alex4.ini file. Should we?

@fbrosson
Copy link
Member

fbrosson commented Jun 21, 2016

@Begasus: May I try to attach master...fbrosson:alex to this PR, therefore detaching master...Begasus:alex4, in case this is not bad practice when done with permission from the author of the PR (which is you for this one)? Reminder: the commit I pushed to my remote branch has your signature, so my name won't even appear in the git history if merged with the squash feature. Of course, you can grab my commit from my remote branch and push it to yours if you like my changes. I can guide you with that if you wish. You can also ask me to put my signature in the commit, or chose to keep your commit, if you prefer.
Needless to say, feedback from anyone is also very welcome ;-) Thanks in advance!

@fbrosson
Copy link
Member

fbrosson commented Jun 21, 2016

Alex also works on x86_64, so I've updated my remote branch master...fbrosson:alex. BTW, I don't have sound working with Alex on any arch, although I do have sound with color-lines on all archies. Yep, I know color-lines is using SDL, unlike Alex.

@korli, @waddlesplash: But since Alex needs lib:lib{dumb,aldmb}_0.9.3 from media-libs/dumb/ and lib:liballeg from media-libs/allegro/ which are both marked as untested on x86_64, shall I also mark allegro-4.4.2.recipe and dumb-0.9.3.recipe as tested on x86_64? If yes, should I do so without a PR, or with a separate PR, or with this PR (in case I'm given green lights to attach my remote branch to this PR) ? Thanls!

@Begasus
Copy link
Contributor Author

Begasus commented Jun 22, 2016

@fbrosson "May I try to attach master...fbrosson:alex to this PR, therefore detaching master...Begasus:alex4, in case this is not bad practice when done with permission from the author of the PR (which is you for this one)? " Sure, I haven't had much time to look into this (and haven't been able to check the x86 arch :)

@fbrosson
Copy link
Member

fbrosson commented Jun 22, 2016

Cool, thanks.
I found http://stackoverflow.com/questions/10081053/how-to-change-the-base-branch-of-a-pull-request and tried it, but it did not work for me. It yelds:

{
  "message": "Not Found",
  "documentation_url": "https://developer.github.com/v3"
}

Maybe it only works for the user who created the PR.
But maybe I can make it work by first assigning this PR to myself. I'll try that. If it does not work then I'll guide you with 3 simple git commands that will do the update.

@fbrosson fbrosson assigned fbrosson and unassigned fbrosson Jun 22, 2016
@fbrosson
Copy link
Member

fbrosson commented Jun 22, 2016

Well, it looks like I can't replace the source branch of this PR. So here are the commands you can type:

git checkout -b newalex master
git pull --ff-only --commit https://github.com/fbrosson/haikuports.git alex
git log | head -6
git push -f --set-upstream origin alex4

Important: You might need refresh you local master with git checkout master and git pull real_origin master before the git checkout -b newalex master.

@waddlesplash
Copy link
Member

waddlesplash commented Jun 22, 2016

@fbrosson It's better to just close this PR and make a new one.

@fbrosson
Copy link
Member

fbrosson commented Jun 23, 2016

@waddlesplash: Well, I'm pretty sure GitHub considers that closing PRs to reopen new ones is not a very good practice, so I think we should let @Begasus update this one.
And I'm sure he'll be able to do so.

@Begasus: Although updating this PR is not that hard, you are of course free to close this PR. But I think you should try to update it. I'm providing 3 step-by-step methods for that. Pick the one you want and just copy/paste the commands.

  • Easy method:
cd /path/to/haikuports/games-puzzle/alex
git checkout alex4
curl -Lo alex4beos-4.recipe.new https://github.com/fbrosson/haikuports/raw/alex4/games-puzzle/alex/alex4beos-4.recipe
mv alex4beos-4.recipe.new alex4beos-4.recipe
git commit --amend alex4beos-4.recipe
git push -f
  • Recommended method, assuming you agree to update your local and remote master:
cd /path/to/haikuports/games-puzzle/alex
git checkout master
git pull real_origin master
git push
git checkout -b newalex master
git pull --ff-only --commit https://github.com/fbrosson/haikuports.git alex
git log | head -6
git push -f --set-upstream origin alex4
  • Alternate method, which will not change your local master (nor your remote master), as it will pull my alex branch into a newalex branch, so this will pull the commit for alex4 as well as all the previous commits that you might have forgotten to pull, and will put all of them to your a newalex branch, but it won't change your master branch.
cd /path/to/haikuports/games-puzzle/alex
git checkout -b newalex master
git pull --ff --commit https://github.com/fbrosson/haikuports.git alex
git rebase master
git log | head -6
git push -f --set-upstream origin alex4

After that you can detele the newalex branch:

git checkout master
git branch -D newalex

BTW, you might wish to keep the newalex branch until we merge this PR, just in case you would like to change the commit message or suggest other changes in the recipe.

@waddlesplash
Copy link
Member

waddlesplash commented Jun 23, 2016

It is a very good practice when the primary author of the PR changes; it is only strongly discouraged when the author is staying the same. And we can do whatever we want anyway
:) So please do that.

@fbrosson
Copy link
Member

fbrosson commented Jun 23, 2016

@waddlesplash: Good remark. You are right. I'll create a new PR.
Hmm, is it OK to keep the current name: games-puzzle/alex/alex4beos-4.recipe ?
Or should I rename the recipe to just alex-4.recipe ?
Or should I rename the directory to alex4beos or something more explicit ?
What about games-puzzle/alex-the-allegator/alex_the_allegator-4.recipe ?
What naming would you chose? (we have to remember that the runtime is called Alex4, however).
I'm OK with the current name. Just asking in case a name change would be recommended.

While at it, since /boot/system/apps is in the PATH, should I add a (relative) symlink from $appsDir/Alex4 to $appsDir/Alex4BeOS/Alex4, just to be able to start the game from command line by typing just Alex4 ?
Thanks!

@pulkomandy
Copy link
Member

pulkomandy commented Jun 23, 2016

As usual, let's follow Gentoo's scheme for naming and categorizing recipes:
http://gpo.zugaina.org/games-arcade/alex4

Since when is it a puzzle game?

@fbrosson
Copy link
Member

fbrosson commented Jun 23, 2016

Since when is it a puzzle game?

;-)
Fixed, see #666.
BTW, thanks for the link. I did not know about that repo.

@Begasus
Copy link
Contributor Author

Begasus commented Jun 25, 2016

I hadn't noticed it in the Gentoo scheme :)
https://packages.gentoo.org/categories/games-arcade
PS, it's also not in the puzzle section, so I stand corrected :)

2016-06-23 17:07 GMT+02:00 Adrien Destugues notifications@github.com:

As usual, let's follow Gentoo's scheme for naming and categorizing recipes:
http://gpo.zugaina.org/games-arcade/alex4

Since when is it a puzzle game?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#631 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/APUDAoiN_NsBabPnbURxe-lKc5jGnvRYks5qOqEfgaJpZM4IzFfD
.

@pulkomandy
Copy link
Member

pulkomandy commented Jun 26, 2016

On Sat, Jun 25, 2016 at 12:58:39AM -0700, Begasus wrote:

I hadn't noticed it in the Gentoo scheme :)
https://packages.gentoo.org/categories/games-arcade
PS, it's also not in the puzzle section, so I stand corrected :)

packages.gentoo.org lists only the main repo from Gentoo ("official"
packages). gpo.zugaina.org also searches several "overlays" or alternate
repos. So it has even more packages.

@Begasus Begasus deleted the Begasus:alex4 branch Dec 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.