libical: Add new recipe #933

scythx · 2016-12-17T14:07:00Z

Adding new recipe for libcal, an implementation of icalendar protocols and data formats.

fbrosson

Since 2.0.0 has a tarball you should use it, instead of the srcGitRev.

And for the other recipe, it should be called libical-2.1.0~git.recipe.
To rename that file:

cd haikuports/dev-libs/libical
git mv libical-2.0.0~.recipe libical-2.1.0~git.recipe
git commit --amend libical-2.0.0~.recipe libical-2.1.0~git.recipe
git push -f

fbrosson · 2016-12-17T16:58:20Z

dev-libs/libical/libical-2.0.0.recipe

+{
+	make install
+
+	prepareInstalledDevelLibs libical libical_cxx libicalss libicalss_cxx libicalvcal


This line is longer than 80 chars:

prepareInstalledDevelLibs libical libical_cxx libicalss libicalss_cxx libicalvcal

So we need to split in at least 2 lines. Shall we split it with one lib per line, like this?

prepareInstalledDevelLibs \ libical \ libical_cxx \ libicalss \ libicalss_cxx \ libicalvcal

fbrosson · 2016-12-17T17:00:03Z

dev-libs/libical/libical-2.0.0~.recipe

+{
+	make install
+
+	prepareInstalledDevelLibs libical libical_cxx libicalss libicalss_cxx libicalvcal


This line is longer than 80 chars:

prepareInstalledDevelLibs libical libical_cxx libicalss libicalss_cxx libicalvcal

So we need to split in at least 2 lines. Shall we split it with one lib per line, like this?

prepareInstalledDevelLibs \ libical \ libical_cxx \ libicalss \ libicalss_cxx \ libicalvcal

scythx · 2016-12-18T00:35:55Z

Actually i don't understand the tarball part but i hope my lastest commit already fix that.

pulkomandy · 2016-12-18T08:41:43Z

dev-libs/libical/libical-2.0.0.recipe

+LICENSE="MPL v1.0
+	GNU LGPL v2.1"
+REVISION="1"
+scrGitRev="6c4af23b0a95fd105f38e879908cbc80390f3716"


This is not needed anymore since you now use the release tarball.

@raefaldhia, the line @pulkomandy is implicitly requesting the removal is the one which defines srcGitRev, of course. -Sorry if this was obvious.)

pulkomandy · 2016-12-18T08:42:18Z

dev-libs/libical/libical-2.0.0.recipe

+scrGitRev="6c4af23b0a95fd105f38e879908cbc80390f3716"
+SOURCE_URI="https://github.com/libical/libical/releases/download/v$portVersion/libical-$portVersion.tar.gz"
+CHECKSUM_SHA256="654c11f759c19237be39f6ad401d917e5a05f36f1736385ed958e60cf21456da"
+SOURCE_DIR="libical-$portVersion"


This is the default SOURCE_DIR pattern, so this line can be removed

pulkomandy · 2016-12-18T08:43:21Z

dev-libs/libical/libical-2.0.0.recipe

+
+PROVIDES="
+	libical$secondaryArchSuffix = $portVersion
+	lib:libical$secondaryArchSuffix


Check if the library has a soname (there is a file called libical.so.1.2.3 and libical.so.1, for example) and add the matching "= 1.2.3 compat >= 1" to the libraries.

According to Gentoo and Arch Linux the sonames for all these libs in release 2.0.0 are:

libical.so.2.0.0 libical_cxx.so.2.0.0 libicalss.so.2.0.0 libicalss_cxx.so.2.0.0 libicalvcal.so.2.0.0

I guess we could therefore use "= $portVersion compat >= 2" for all the lib:libical* lines in PROVIDES and devel:libical* lines in BUILD_REQUIRES. Would that be OK to use that? My understanding is that we should avoid this in the general case, but since upstream is using these sonames in sync with the version string, it would be cool to use $portVersion here.

BTW, maybe we should drop libical-2.1.0~git.recipe and only keep libical-2.0.0.recipe.

@fbrosson yeah i think so, libical-2.1.0~git.recipe requre glib 2.38 but when i see in glib recipe it doesn't provide something like lib:glib or lib:libglib current libical-2.1.0~git.recipe actually 2.0.50 https://github.com/libical/libical/blob/master/CMakeLists.txt#L102

pulkomandy · 2016-12-18T08:44:26Z

dev-libs/libical/libical-2.1.0~.recipe

@@ -0,0 +1,83 @@
+SUMMARY="An implementation of iCalendar protocols and data formats"


Is the ~ at the end of the package name intentional? It looks like a strange version pattern.

Oppss it should be libical-2.1.0~git missing the git

Yep, it is intentional, but @raefaldhia just forgot to rename the file to libical-2.1.0~git.recipe (which is, if I'm not mistaken, the convention to use when we know that 2.1.0 has not been released yet, and that the current git commit will be part of that release when it becomes available.)

pulkomandy · 2016-12-18T08:46:03Z

dev-libs/libical/libical-2.1.0~.recipe

+SECONDARY_ARCHITECTURES="!x86_gcc2 ?x86"
+
+PROVIDES="
+	libical$secondaryArchSuffix = $portVersion


Are this and the libical 2 compatible at the ABI level?
If they are not, you need to have different package names (libical and libical2) and proper sonames/compat declarations (see my comments on the other recipe). Otherwise, the two packages will conflict and it will not be possible to install them side by side.
You can look at the libpng recipes for an example of this.

scythx · 2016-12-18T10:49:53Z

Updated.

fbrosson · 2016-12-18T10:56:28Z

dev-libs/libical/libical-2.0.0.recipe

+calendar clients can communicate with calendar servers so users can store \
+their calendar data and arrange meetings with other users."
+HOMEPAGE="http://libical.github.io/libical/"
+COPYRIGHT="Eric Busboom"


It would be nice to add the copyright years, using this format:

COPYRIGHT="20??-201? Eric Busboom"

fbrosson

The recipe is almost OK. Changes needed:

Fix typo in copyright year.
Move $libDir/cmake/libical to the devel pacakge.

I'm having some trouble with "haikuporter --test --get-dependencies libical_x86" and am trying to see if we can make the tests succed.
(I also wish to figure out if we really need cmd:perl in BUILD_PREREQUIRES.)

BTW, sorry for the delay ;-)

fbrosson · 2016-12-18T15:18:43Z

dev-libs/libical/libical-2.0.0.recipe

+calendar clients can communicate with calendar servers so users can store \
+their calendar data and arrange meetings with other users."
+HOMEPAGE="http://libical.github.io/libical/"
+COPYRIGHT="1999-201? Eric Busboom"


Please replace "201?" by "2015" (since 2.0.0 was released last year.)

fbrosson · 2016-12-18T15:21:52Z

dev-libs/libical/libical-2.0.0.recipe

+	fixPkgconfig
+
+	packageEntries devel \
+		$developDir


Please replace

packageEntries devel \ $developDir

by

packageEntries devel \ $developDir \ $libDir/cmake

scythx · 2016-12-18T23:05:19Z

@fbrosson

I also wish to figure out if we really need cmd:perl in BUILD_PREREQUIRES.

https://github.com/libical/libical/blob/master/Install.txt#L15

I have no idea with the test i'll try to set threadlibs and iculibs

fbrosson · 2016-12-18T23:46:30Z

OK, thanks for the info about perl being required. (I had skipped that!!!)

Regarding icu, this is what I did to find out which libs it can use:

I added this to BUILD_REQUIRES:

	devel:libicudata$secondaryArchSuffix
	devel:libicui18n$secondaryArchSuffix
	devel:libicuio$secondaryArchSuffix
	devel:libicule$secondaryArchSuffix
	devel:libiculx$secondaryArchSuffix
	devel:libicutest$secondaryArchSuffix
	devel:libicutu$secondaryArchSuffix
	devel:libicuuc$secondaryArchSuffix

I called "hp -S libicall_x86," which told me that libicui18n.so and libicuuc.so were required.
I then removed all the devel:libicu* lines I had previously added to BUILD_REQUIRES and only kept these 2::

	devel:libicui18n$secondaryArchSuffix
	devel:libicuuc$secondaryArchSuffix

I added this to REQUIRES:

	lib:libicui18n$secondaryArchSuffix
	lib:libicuuc$secondaryArchSuffix

This builds fine for x86 secondary arch on x86_gcc2 and should also build fine on x86 primary arch, of course.
(I need to check it on x86_64.)

Regarding the tests, I found an error message about a missing "bswap_32" symbol but did not google it yet.

EDIT: Maybe it was not "bswap_32" but "swapb_32"; I can't remember exactly. I would need to retest.

fbrosson · 2016-12-18T23:56:09Z

I forgot to mention that I also added cmd:icu_config$secondaryArchSuffix to BUILD_PREREQUIRES.
BTW, the tests still fail, even with libicu :-(

scythx · 2016-12-19T00:37:10Z

Here's refference linux build, some test actually success without libicu
https://gist.github.com/raefaldhia/fe638e77442df7f19d4b58badceff4a2

scythx · 2016-12-19T00:50:36Z

Maybe you're right, it caused by bswap, when i try to run the regression manually i got this result:

~/haiku/haikuports/dev-libs/libical/work-x86-2.0.0/sources/libical-2.0.0/src/test> ./regression
runtime_loader: /boot/system/lib/x86/libical.so.2.0.0: Could not resolve symbol 'bswap_32'
resolve symbol "bswap_32" returned: -2147478780
runtime_loader: /boot/system/lib/x86/libical.so.2.0.0: Troubles relocating: Symbol not found
~/haiku/haikuports/dev-libs/libical/work-x86-2.0.0/sources/libical-2.0.0/src/test> ./timezones
runtime_loader: /boot/system/lib/x86/libical.so.2.0.0: Could not resolve symbol 'bswap_32'
resolve symbol "bswap_32" returned: -2147478780
runtime_loader: /boot/system/lib/x86/libical.so.2.0.0: Troubles relocating: Symbol not found
~/haiku/haikuports/dev-libs/libical/work-x86-2.0.0/sources/libical-2.0.0/src/test>

scythx · 2016-12-19T01:08:17Z

I found interesting stuff, replacing bswap_32 macro inside icaltz-util.c make some test passed but
2 test, 2 test failed, 3 test passed.

I replaced the macro with:
http://svn.openmoko.org/trunk/src/host/qemu-neo1973/bswap.h

I don't know if Haiku already have bswap.h or not.

but libaviutil have the bswap.h so we need to find the linker name to pass something like -libaviutilto makefile

Ignore libaviutil,

haikuports/net-wireless/wpa_supplicant/patches/wpa_supplicant-2.0.patch

Line 703 in 75a3cf5

+#define bswap_16 __swap_int16

fbrosson · 2016-12-19T02:06:44Z

man bswap tells bswap_32 is in byteswap.h, so I looked for it in:/system/develop/headers and found it is shipped in.. calc_devel, produced by one of the recipes you wrote! Isn't this amazing?

Well, it defines several macros, but with different names. Moreover, it would not be appropriate to make libical build-depend on calc_devel.
So I guess your macro is the best option.
Can you include it in your PR, as well as the ICU stuff?

scythx · 2016-12-19T02:16:19Z

amazing xd, I couldn't understand why the 4 tests still fail, when i checked it's still aboult bswap_32.

scythx · 2016-12-19T07:05:47Z

My lastest commit should work. do you still facing something about bswap_32 ?

fbrosson

Minor changes (regarding the dependency on ICU) that should either be all applied or all ignored.

fbrosson · 2016-12-19T07:54:04Z

dev-libs/libical/libical-2.0.0.recipe

+	"
+REQUIRES="
+	haiku$secondaryArchSuffix
+	icu$secondaryArchSuffix


I think you should replace:

icu$secondaryArchSuffix

by

lib:libicui18n$secondaryArchSuffix lib:libicuuc$secondaryArchSuffix

But maybe we should wait for a confirmation from another contributor.

fbrosson · 2016-12-19T07:56:24Z

dev-libs/libical/libical-2.0.0.recipe

+
+BUILD_REQUIRES="
+	haiku${secondaryArchSuffix}_devel
+	icu${secondaryArchSuffix}_devel


I think you should replace:

icu${secondaryArchSuffix}_devel

by

devel:libicui18n$secondaryArchSuffix devel:libicuuc$secondaryArchSuffix

But maybe we should wait for a confirmation from another contributor.

fbrosson · 2016-12-19T08:00:17Z

dev-libs/libical/libical-2.0.0.recipe

+	cmd:gcc$secondaryArchSuffix
+	cmd:make
+	cmd:perl
+	cmd:wget


I think we also need cmd:icu_config$secondaryArchSuffix in BUILD_PREREQUIRES.

BTW, do we really need cmd:wget ?
Ah, maybe it is required for the test...

@fbrosson oopps forget to remove it. i used that before, but now the recipe doesn't need it.

Cooll.
BTW, congratulations for having solved the bswap_32 issue!

Adding new recipe for libcal, an implementation of icalendar protocols and data formats.

fbrosson · 2016-12-19T08:47:26Z

Thanks for the update. I'll test it right now.

BTW, on Arch Linux they also pass "-DCMAKE_BUILD_TYPE=Release" to cmake.
Maybe we could do this too? ;-)

fbrosson · 2016-12-19T09:35:14Z

The recipe looks OK, although 4 tests are failing on my system (like on yours):

The following tests FAILED:
	  1 - regression (Failed)
	  2 - recur (Failed)
	  4 - testvcal (OTHER_FAULT)
	  6 - builtin_timezones (Failed)
Errors while running CTest

(testvcal even causes a crash.)

I think we can merge this as is, and if anyone ever notices that libical is not working then we can always either try to fix it or prepend a "?" to each arch in this recipe.
(So let's just forget about the "-DCMAKE_BUILD_TYPE=Release" at the moment ;-)

scythx · 2016-12-19T09:52:27Z

Crash caused by exception maybe,testvcal give exception on linux, recur fail on linux. should i submit task in my gci task-instances?
actually https://github.com/haikuports/haikuports/pull/933/files#diff-2f6381c793bd9ea937a061556fbe38d7R63 affect regression(without it, regression crash) and builtin_timezone

fbrosson · 2016-12-19T10:06:15Z

If you would like to dig the issue about the tests, that's OK for me, but I guess you should probably take some rest and look for other recipes to write, while the contest is open ;-)

So if you publish this task in your gci task-instances then I will validate it!

I'll wait for @pulkomandy's approval before I merge, but I guess there won't be any changes.

pulkomandy · 2016-12-19T10:11:59Z

If the library is actually named libical.so-2.0.0 with a soname named libical.so-2, then yes, it is fine. Otherwise, you will need to remove the portversion/compat for all the lib: entries in provides. Can't check myself, I'm at work with no access to Haiku machine.

scythx · 2016-12-19T10:20:09Z

So the when library named libical.so.2.0.0(with dot) i should remove the portversion/compat?

pulkomandy · 2016-12-19T10:22:32Z

Oops, you're right, it is a dot. So if you have libical.so.2.0.0 and libical.so.2, you can keep it the way it is now.

fbrosson · 2016-12-19T10:23:28Z

@pulkomandy: we get nice filenames:

package list -p /system/packages/libical_x86-2.0.0-1-x86_gcc2.hpkg  | grep ^lib/
lib/x86
lib/x86/libical.so.2
lib/x86/libical.so.2.0.0
lib/x86/libical_cxx.so.2
lib/x86/libical_cxx.so.2.0.0
lib/x86/libicalss.so.2
lib/x86/libicalss.so.2.0.0
lib/x86/libicalss_cxx.so.2
lib/x86/libicalss_cxx.so.2.0.0
lib/x86/libicalvcal.so.2
lib/x86/libicalvcal.so.2.0.0

@raefaldhia: the current declarations seem OK to me.

fbrosson reviewed Dec 17, 2016

View reviewed changes

scythx force-pushed the libical branch from ba6e315 to 07641e2 Compare December 18, 2016 00:35

pulkomandy requested changes Dec 18, 2016

View reviewed changes

scythx force-pushed the libical branch from 07641e2 to fa5e096 Compare December 18, 2016 10:22

fbrosson reviewed Dec 18, 2016

View reviewed changes

scythx force-pushed the libical branch from fa5e096 to 3b595b5 Compare December 18, 2016 12:04

fbrosson requested changes Dec 18, 2016

View reviewed changes

scythx force-pushed the libical branch 2 times, most recently from 8433b70 to d9ae2ed Compare December 19, 2016 06:35

fbrosson reviewed Dec 19, 2016

View reviewed changes

libical: Add new recipe

cb903f9

Adding new recipe for libcal, an implementation of icalendar protocols and data formats.

scythx force-pushed the libical branch from d9ae2ed to cb903f9 Compare December 19, 2016 08:36

fbrosson approved these changes Dec 19, 2016

View reviewed changes

pulkomandy approved these changes Dec 19, 2016

View reviewed changes

fbrosson merged commit 643a3b3 into haikuports:master Dec 19, 2016

scythx deleted the libical branch December 19, 2016 23:03

		@@ -0,0 +1,83 @@
		SUMMARY="An implementation of iCalendar protocols and data formats"

libical: Add new recipe #933

libical: Add new recipe #933

Conversation

scythx commented Dec 17, 2016

fbrosson left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scythx commented Dec 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scythx commented Dec 18, 2016

Choose a reason for hiding this comment

fbrosson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scythx commented Dec 18, 2016 • edited

fbrosson commented Dec 18, 2016 • edited

fbrosson commented Dec 18, 2016

scythx commented Dec 19, 2016

scythx commented Dec 19, 2016

scythx commented Dec 19, 2016 • edited

fbrosson commented Dec 19, 2016

scythx commented Dec 19, 2016 • edited

scythx commented Dec 19, 2016 • edited

fbrosson left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fbrosson commented Dec 19, 2016

fbrosson commented Dec 19, 2016

scythx commented Dec 19, 2016 • edited

fbrosson commented Dec 19, 2016

pulkomandy commented Dec 19, 2016

scythx commented Dec 19, 2016 • edited

pulkomandy commented Dec 19, 2016

fbrosson commented Dec 19, 2016

fbrosson left a comment •

edited

scythx commented Dec 18, 2016 •

edited

fbrosson commented Dec 18, 2016 •

edited

scythx commented Dec 19, 2016 •

edited

scythx commented Dec 19, 2016 •

edited

scythx commented Dec 19, 2016 •

edited

fbrosson left a comment •

edited

scythx commented Dec 19, 2016 •

edited

scythx commented Dec 19, 2016 •

edited