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

irssi: add recipe for version 0.8.19 #553

Closed
wants to merge 1 commit into from

Conversation

@pisculichi
Copy link
Contributor

pisculichi commented Apr 8, 2016

No description provided.

@korli
Copy link
Contributor

korli commented Apr 9, 2016

Hi,
should the PR for irssi 0.8.17 be dismissed?

@pisculichi
Copy link
Contributor Author

pisculichi commented Apr 9, 2016

Yes, this is for the last versión.

BUILD_PACKAGED_ACTIVATION_PHASE="INSTALL"
STATUS_HAIKU="stable"
PROVIDES="
botti$secondaryArchSuffix = $portVersion

This comment has been minimized.

Copy link
@korli

korli Apr 30, 2016

Contributor

why providing botti? this line should be dropped.

This comment has been minimized.

Copy link
@pisculichi

pisculichi May 2, 2016

Author Contributor

Droped!

@ailin-nemui
Copy link

ailin-nemui commented Apr 30, 2016

being totally ignorant about haiku, I would like you to double check if those "Move variable declarations" patches are all required. Because ANSI C does not demand variables only at function top at all

@fbrosson
Copy link
Member

fbrosson commented Apr 30, 2016

@pisculichi, I can confirm what @ailin-nemui wrote because the gcc2 we have in Haiku for x86_gcc2 does not require that variables be declared in the begining of a function. It only requires that all declarations in a «{}» bloc appear before the very first statement of that bloc. So you don't need to move declarations that appear inside «{}» blocs if these declarations are already grouped in the begining of its deepest bloc. This means that if you see any declaration mixed with code then you only need to move them up near the closest «{» that contains it.
What happens very often is that you have to split the declaration and its initialisation if that initialisation is done with a function call.

char *foo = strdup("foo");
char *bar = strdup("bar");

would have to be changed to:

char *foo;
char *bar;
foo = strdup("foo");
bar = strdup("bar");

or, even better:

char *foo, *bar;
foo = strdup("foo");
bar = strdup("bar");

BTW, thanks @ailin-nemui for spotting this.

@ailin-nemui
Copy link

ailin-nemui commented Apr 30, 2016

you're welcome to submit genuine ANSI C fixes upstream

@fbrosson
Copy link
Member

fbrosson commented Apr 30, 2016

@ailin-nemui: That's cool, thanks!
@pisculichi: I'll let you take care of it, of course.

@pisculichi pisculichi mentioned this pull request May 2, 2016
@pisculichi pisculichi force-pushed the pisculichi:irssi-0.8.19 branch from 50caf6a to 873ac43 May 2, 2016
@pisculichi
Copy link
Contributor Author

pisculichi commented May 2, 2016

@ailin-nemui @fbrosson you are right I moved all declarations when I saw errors at compiling. I update this commit with your recommendations.

@pisculichi pisculichi force-pushed the pisculichi:irssi-0.8.19 branch from 873ac43 to 685a35a May 2, 2016
@mmuman
Copy link
Member

mmuman commented May 3, 2016

@ailin-nemui @fbrosson as I already said, those are C89 fixes, because that's all gcc2 supports. But it's not strictly required for irssi since it doesn't provide any library so we don't really have any dependency on it, so we can build with gcc4 (5 now).

@fbrosson
Copy link
Member

fbrosson commented May 3, 2016

@mmuman: I though it was good practice to try and make all recipes build with gcc2 when that was either easy or essential. But now I just understood your remark regarding those packages that do not provide any library.
There is, however, a small reason to also convert those packages to x86_gcc2: fewer dependencies. And, if I'm not mistaken, such packages may also be used on BeOS if converted to gcc2.
Hence this question: Now that a gcc2 patch is ready for this package, would it be OK, if it works perfectly, to accept it?
@pisculichi: I read your gcc2 patches and everything seems OK to me. I was planning to build it but did not try it yet. I will do so and I hope your gcc2 patches will be accepted for a merge. I hope you won't be angry against me if the gcc2 patches do not get merged, because I was not aware of the rule when I wrote my first message in this PR...

@pisculichi
Copy link
Contributor Author

pisculichi commented May 3, 2016

@fbrosson no problem. I did it to learn haikuports, git, and contribute in someway. I understand that support ANSI C in 2016 maybe has not sense.
Anyway, thanks all for the comments =)

@fbrosson
Copy link
Member

fbrosson commented May 3, 2016

Thanks @pisculichi for your kind answer.
Hmm, the patch @pisculichi sent to upstream has just been accepted and merged by @ailin-nemui. That's cool. Thanks @ailin-nemui!

@fbrosson
Copy link
Member

fbrosson commented May 4, 2016

The following bloc allows to reference a tarball of the snapshot just after the commit that adds the gcc2 patches:

srcGitRev="1cfec5f63d4adf32117f68638fb35802146e2784"
SOURCE_URI="https://github.com/irssi/irssi/archive/$srcGitRev.tar.gz"
CHECKSUM_SHA256="e4118fedeee70133b4aab70406c30be107cc0078c4d5c509e2b068277284d88a"
SOURCE_FILENAME="irssi-$srcGitRev.tar.gz"
SOURCE_DIR="irssi-$srcGitRev"

I don't know, however, if it's OK to do so without renaming the recipe to, say, irssi-0.8.19git.recipe (or maybe irssi-0.8.20git.recipe or something similar) to make it clear that it's not exactly 0.8.19 but a snapshot between 0.8.19 and 0.8.20.
(BTW, I'm not sure I used the recomended order for CHECKSUM_SHA256, SOURCE_FILENAME and SOURCE_DIR. So please correct me if that's not the most used order.)

@mmuman
Copy link
Member

mmuman commented May 5, 2016

@fbrosson it's ok to keep the release + gcc2 patch until the next one is out. If you want a git recipe you'd rather use 0.8.19_git as naming I think.

@fbrosson
Copy link
Member

fbrosson commented May 5, 2016

OK, thanks. BTW, I noticed that there are indeed several recipes with "_git", and only 2 with "~git", but these two are quite recent, so I though there was a little change in the naming conventions.
Anyway, I'll let @pisculichi drive this PR cause I don't plan to add a recipe with a snapshot.

@waddlesplash
Copy link
Member

waddlesplash commented Jun 18, 2016

Bump.

@pisculichi
Copy link
Contributor Author

pisculichi commented Jun 19, 2016

I forgot about this, sorry.
Should I improve the changes that @fbrosson told me (modify and rename irssi-0.8.19_git)?

@waddlesplash
Copy link
Member

waddlesplash commented Jun 19, 2016

Yes, please.

@pisculichi pisculichi force-pushed the pisculichi:irssi-0.8.19 branch from 685a35a to f86d23e Jun 19, 2016
@@ -0,0 +1,68 @@
LICENSE="GNU GPL v2"
COPYRIGHT="1999-2015 Timo Sirainen"

This comment has been minimized.

ARCHITECTURES="x86_gcc2 x86"
SECONDARY_ARCHITECTURES="x86"
BUILD_PACKAGED_ACTIVATION_PHASE="INSTALL"
STATUS_HAIKU="stable"

This comment has been minimized.

Copy link
@waddlesplash

waddlesplash Jun 19, 2016

Member

This and BUILD_PACKAGED_ACTIVATION_PHASE are no longer used.

irssi-ncurses.patch
"

BUILD() {

This comment has been minimized.

Copy link
@waddlesplash

waddlesplash Jun 19, 2016

Member

Braces go on next line.

@pisculichi pisculichi force-pushed the pisculichi:irssi-0.8.19 branch from f86d23e to e40cde7 Jun 19, 2016
@waddlesplash
Copy link
Member

waddlesplash commented Jul 22, 2016

Manually merged. Sorry that took so long!

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

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