Organicmaps fixes #185
Open
Organicmaps fixes #185
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stkw0
reviewed
May 6, 2024
|
Can organicmaps and the other ebuild be versioned instead of being live version only? I think organicmaps has tags/releases. |
fast_double_parser cannot be a released version, yet. There exist releases but only in the git version an install target exists (the older versions just compile the sources but do not install any header/library), see lemire/fast_double_parser@2a3319d. For organicmaps, there could be a release version. I will try to add it. |
|
The last version is from 2022, I guess it would make sense to open an issue upstream to see how is the situation and if a new version could be released. If not it would make more sense to git the snapshot of a given date and use the date as version in the ebuild. |
Signed-off-by: Gerion Entrup <gerion.entrup@flump.de>
|
Thankfully, @lemire released a new version, so organicmaps now depends on that version. |
|
|
||
| BDEPEND="" | ||
| DEPEND="" | ||
| RDEPEND="" |
There was a problem hiding this comment.
I think you can drop these empty vars.
There was a problem hiding this comment.
I just copied the upstream ebuild and therefore left it as is.
There was a problem hiding this comment.
In any case, please remove these empty variables
| fi | ||
| fi | ||
|
|
||
| rmdir "${S}/extern/ftest" || die |
There was a problem hiding this comment.
Is || die already integrated inside rmdir?
There was a problem hiding this comment.
No (rmdir is just a coreutils(?) program).
| fi | ||
|
|
||
| rmdir "${S}/extern/ftest" || die | ||
| ln -s ../../ftest "${S}/extern/ftest" || die |
There was a problem hiding this comment.
We need this only when USE test?
|
|
||
| +# Add protobuf library. | ||
| +add_subdirectory(protobuf) | ||
| + |
There was a problem hiding this comment.
Please add a comment here - why do we need this (why not fixed in upstream), and when we can remove this patch.
There was a problem hiding this comment.
The comment is in the ebuild itself, there is the appropriate PR linked that is needed in organicmaps to make this patch obsolete.
|
Big thanks for fixes, today or tomorrow I will try to run this. |
A version >=4 is an organicmaps dependency. Signed-off-by: Gerion Entrup <gerion.entrup@flump.de>
There are releases but none of them seem to be downloadable for the desktop version. One could tie this to a specific commit using EGIT_COMMIT and make it a 20240504 or similar like the ones found here: https://github.com/organicmaps/organicmaps/releases/ |
The releases are linked to a specific git commit and the assets contain a source code tarball besides the APK. I would just use this as "desktop release" (Gentoo needs the source tarball anyway). |
|
Cannot compile organicmaps, logs ends with: |
- adjust patches. They depend on organicmaps/organicmaps#7982 organicmaps/organicmaps#6310 - fix dependencies. OrganicMaps has more external libs now. Moreover all dynamically linked libraries were checked and added as deps. - don't install test targets Signed-off-by: Gerion Entrup <gerion.entrup@flump.de>
Strange error and should be unrelated to my changes. CMake does not find a compiler with pthread support. Is your gcc compiled with the openmp useflag? |
Yes. |
Using the source tarballs is not easy. I came up with this: PYTHON_COMPAT=( python3_{7..12} )
inherit git-r3 python-r1 xdg cmake
# this URL is to make the tests compile since organicmaps usually dynamically clones the repo
# maybe a better way would be to skip the test
EGIT_WORLD_FEED_REPO_URI="https://github.com/${PN}/world_feed_integration_tests_data.git"
if [[ ${PV} == 9999 ]]; then
EGIT_REPO_URI="https://github.com/${PN}/${PN}.git"
# organicmaps gets more and more system libraries, we use as many
# as currently possible, use submodules for the rest
EGIT_SUBMODULES=(
3party/just_gtfs
3party/protobuf/protobuf # wait for https://github.com/organicmaps/organicmaps/pull/6310
3party/fast_obj
)
else
# organicmaps itself and the bundled dependencies
SRC_URI="
https://github.com/organicmaps/organicmaps/archive/refs/tags/${PV}-9-android.tar.gz -> ${P}.tar.gz
https://github.com/organicmaps/just_gtfs/archive/7516753825500f90ac2de6f18c256d5abec1ff33.tar.gz -> just_gtfs-75167538.tar.gz
https://github.com/organicmaps/protobuf/archive/bda4bb90688a36e1311d798030894dcd5f6105ef.tar.gz -> organicmaps-protobuf-bda4bb906.tar.gz
https://github.com/thisistherk/fast_obj/archive/724e09ff1e82679eeab5ae56652ecc3be593b5b0.tar.gz -> organicmaps-fast_obj-724e09ff1.tar.gz
"
fiExplanation:
However, the whole ebuild does not look clean for me, here. OrganicMaps has done a lot in the past to unbundle dependencies. I would just wait some time to get a cleaner ebuild for a specific release. |
Hmm, I don't have a good idea. Can you put this pthread test application into a file ( |
|
|
MrRoy
suggested changes
May 13, 2024
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <!DOCTYPE pkgmetadata SYSTEM "https://www.gentoo.org/dtd/metadata.dtd"> | ||
| <pkgmetadata> | ||
| <upstream> |
There was a problem hiding this comment.
Can you add yourself as maintainer?
You will have to create an account on bugs.gentoo.org as well with the same email.
This will allow you to receive notification emails when bugs are reported for this package.
Alternatively, you could put @vitaly-zdanevich as maintainer instead if he agrees to it.
There was a problem hiding this comment.
Ok, I will do it.
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <!DOCTYPE pkgmetadata SYSTEM "https://www.gentoo.org/dtd/metadata.dtd"> | ||
| <pkgmetadata> | ||
| <upstream> |
There was a problem hiding this comment.
Same comment for this, regarding adding a maintainer
|
|
||
| BDEPEND="" | ||
| DEPEND="" | ||
| RDEPEND="" |
There was a problem hiding this comment.
In any case, please remove these empty variables
|
|
||
| LICENSE="Boost-1.0" | ||
| SLOT="0" | ||
| KEYWORDS="~amd64 ~arm ~arm64 ~loong ~ppc ~ppc64 ~riscv ~sparc ~x86" |
There was a problem hiding this comment.
Please don't add every architecture ever. Usually just add the one(s) you actually tested this on (typically ~amd64).
More keywords can be requested by users (as bugs on bugs.gentoo.org) if they test on different archs and reports that it works fine.
|
But still, I cannot compile this :( |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@vitaly-zdanevich