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

New recipe: MEOS #9453

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

New recipe: MEOS #9453

wants to merge 17 commits into from

Conversation

evetion
Copy link
Contributor

@evetion evetion commented Sep 19, 2024

MEOS (Mobility Engine, Open Source) is a C library and its associated API for manipulating temporal and spatiotemporal data. It is the core component of MobilityDB, an open source geospatial trajectory data management & analysis platform built on top of PostgreSQL and PostGIS.

See https://libmeos.org/

edit: This release now points to the release candidate 1 release (as it requires no patches) to build. Is it ok to set the version number to v1.2.0 already and later at the actual release just rerunning it with the new commits (producing a jll with the +1 postfix)?

@evetion
Copy link
Contributor Author

evetion commented Sep 19, 2024

This fails on PROJ not being found, because:
┌ Warning: Dependency Proj_jll does not have a mapping for artifact Proj for platform x86_64-linux-gnu-libgfortran3-cxx03.

However, I'm not sure how/why the libgfortran(3) gets injected, and why PROJ_jll has no such mapping, whereas GEOS_jll with a similar mapping does.

edit: @giordano Could you help me out here? I'm overlooking something, but can't pinpoint it.

@evetion evetion marked this pull request as ready for review September 24, 2024 11:50
M/MEOS/build_tarballs.jl Outdated Show resolved Hide resolved
@evetion evetion mentioned this pull request Sep 26, 2024
@imciner2
Copy link
Member

edit: This release now points to the release candidate 1 release (as it requires no patches) to build. Is it ok to set the version number to v1.2.0 already and later at the actual release just rerunning it with the new commits (producing a jll with the +1 postfix)?

I'd suggest not doing that. The postfix is not really liked by the general registry, so while we use it for the individual builds here, it would be better to not rely on it for this.

How soon will upstream release 1.2.0?

M/MEOS/build_tarballs.jl Outdated Show resolved Hide resolved
Co-authored-by: Ian McInerney <[email protected]>
@evetion
Copy link
Contributor Author

evetion commented Sep 27, 2024

edit: This release now points to the release candidate 1 release (as it requires no patches) to build. Is it ok to set the version number to v1.2.0 already and later at the actual release just rerunning it with the new commits (producing a jll with the +1 postfix)?

I'd suggest not doing that. The postfix is not really liked by the general registry, so while we use it for the individual builds here, it would be better to not rely on it for this.

How soon will upstream release 1.2.0?

Fair enough, let's wait for the 1.2.0 release, but I'm not sure how soon that will be. For now, this is also still blocked on Windows (Error: incorrect register '%eax' used with 'q' suffix), for which I made an upstream issue.

@giordano giordano marked this pull request as draft October 5, 2024 22:27
@mschoema
Copy link

We just produced the v1.2.0 release of MEOS. Feel free to let us know if you encounter any issues with it.

@evetion evetion marked this pull request as ready for review October 20, 2024 14:28

# Collection of sources required to complete build
sources = [
GitSource("https://github.com/MobilityDB/MobilityDB.git", "60048b5b4b7ce2f7560c024d1af024db73b3bd5b")
Copy link
Member

Choose a reason for hiding this comment

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

I am confused: why is the recipe called MEOS build it builds a project called "MobilityDB". I just went to https://github.com/MobilityDB/MobilityDB and it doesn't mention "MEOS" in the README.md (though there is a subdirectory called meos).

So is MEOS a subset of MobilityDB? But building MobilityDB for some reason... only builds and installs MEOS, not the whole thing?

Perhaps it would be good to add a few comments to explain the situation? Or rename this recipe?

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 that's explained in the PR description (and on the linked website). I will add a similar comment with the link to the source here.

Copy link
Contributor

Choose a reason for hiding this comment

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

"Mobility Engine Open Source" also does not seem to be the obvious search hit for "MeOS" on Google.

Recommendation: Choose a longer, more explicit name, e.g., MobilityEngineOpenSource.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed that Google doesn't help (right now), but I'm not sure if that's a good requirement? Eigen, much more famous, doesn't show up in my search results either. I suppose we tend to follow how the external library calls itself, which is MEOS. Is there a naming guide for when we deviate from the library name itself?

FWIW, it is supposed to mirror similarly named software (also recipes here), namely GEOS (but GDAL and PROJ also come to mind).

M/MEOS/build_tarballs.jl Outdated Show resolved Hide resolved
Comment on lines +23 to +24
-DHAVE_X86_64_POPCNTQ_EXITCODE="FAILED_TO_RUN" \
-DHAVE_X86_64_POPCNTQ_EXITCODE__TRYRUN_OUTPUT=
Copy link
Member

Choose a reason for hiding this comment

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

Why? This is such an old instruction which should be moderately safe to assume to be available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently it's not the instruction, but the TRY_RUN statement is not allowed in cross-compile mode:

[14:57:59] CMake Error: TRY_RUN() invoked in cross-compiling mode, please set the following cache variables appropriately:
[14:57:59]    HAVE_X86_64_POPCNTQ_EXITCODE (advanced)
[14:57:59]    HAVE_X86_64_POPCNTQ_EXITCODE__TRYRUN_OUTPUT (advanced)
[14:57:59] For details see /workspace/srcdir/MobilityDB/build/TryRunResults.cmake

Copy link
Member

Choose a reason for hiding this comment

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

but the TRY_RUN statement is not allowed in cross-compile mode:

I know. But I expect that try-run is running a check whether the instruction is available. Which of course could also be done with a simple compiler check instead of a try-run.

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 just workarounded the error with the instruction CMake gave me (I'm not affiliated with MobilityDB/MEOS so I can't answer the why, but I'm happy to make an upstream issue). AFAIK there's no code in MEOS actually making use of this check (anymore MobilityDB/MobilityDB#610).

Copy link
Member

Choose a reason for hiding this comment

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

We use a cross-compilation environment, so any try_run call in a CMake script will basically be pointless. From the CMake docs (https://cmake.org/cmake/help/latest/command/try_run.html#behavior-when-cross-compiling):

When cross compiling, the executable compiled in the first step usually cannot be run on the build host. The try_run command checks the CMAKE_CROSSCOMPILING variable to detect whether CMake is in cross-compiling mode. If that is the case, it will still try to compile the executable, but it will not try to run the executable unless the CMAKE_CROSSCOMPILING_EMULATOR variable is set. Instead it will create cache variables which must be filled by the user or by presetting them in some CMake script file to the values the executable would have produced if it had been run on its actual target platform. These cache entries are:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants