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

Try to make cmake work again #8201

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

raedrizqie
Copy link

Built successfully on MinGW-w64 systems (mingw64, ucrt64, clang64):
https://github.com/msys2/MINGW-packages/actions/runs/10094396971?pr=21492

@asfernandes asfernandes changed the title Try to make it work again Try to make cmake work again Aug 2, 2024
set(FB_IPC_NAME "FIREBIRD")
endif()

if (MINGW)
set(FB_BINDIR "${FB_PREFIX}/bin")
Copy link
Member

Choose a reason for hiding this comment

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

My problem with this is that it makes a windows build to use fixed instead of relocatable paths.
In the past mingw was a Windows build tool, but then seems it (together with msys) become a packaging environment too. I'm not sure just a build using mingw should follow its package conventions.

I never used cmake with Firebird, but I like it (or dislike less than the configure+make approach) and maybe we can have a cmake build with mingw compatible with the current Windows MSVC build.

Don't see this comment as an objection, as a short term solution it may be ok for me.

Does it also work using MSVC?

Copy link
Author

Choose a reason for hiding this comment

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

My problem with this is that it makes a windows build to use fixed instead of relocatable paths.

FB_PREFIX is only used on build-time. The real final installation location is defined by FB_INSTALL_PREFIX (at least for mingw). MinGW does not support binreloc, so we add pathtools.cpp and pathtools.h to get relocatable paths.

In the past mingw was a Windows build tool, but then seems it (together with msys) become a packaging environment too. I'm not sure just a build using mingw should follow its package conventions.

MinGW generally follows FHS convention.. but, if you guys prefer for it to follow MSVC directory structure, im ok with that. MSys2 can always patch it downstream.

I never used cmake with Firebird, but I like it (or dislike less than the configure+make approach) and maybe we can have a cmake build with mingw compatible with the current Windows MSVC build.

Don't see this comment as an objection, as a short term solution it may be ok for me.

Great to hear that :)

Does it also work using MSVC?

I never tried this setup on msvc. sorry..

@hvlad
Copy link
Member

hvlad commented Aug 2, 2024

I see no need and no way to use MinGW as developer platform on Windows.
At least as long as we have free VS and it satisfy our needs.

MinGW could be used by anybody else for any other purposes, of course, but not me and not as main tool for developer.

@raedrizqie
Copy link
Author

I see no need and no way to use MinGW as developer platform on Windows. At least as long as we have free VS and it satisfy our needs.

MinGW could be used by anybody else for any other purposes, of course, but not me and not as main tool for developer.

Fair enough for me :)

@AlexPeshkoff
Copy link
Member

Did you check cmake build on linux after mentioned changes?

@raedrizqie
Copy link
Author

Did you check cmake build on linux after mentioned changes?

No, I didnt.. I only have access to windows. I am hoping someone else could help to improve it for other platform.

Copy link
Member

@AlexPeshkoff AlexPeshkoff left a comment

Choose a reason for hiding this comment

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

I myself never use cmake build and not ready to fix/check it on linux. I think in current state patch can't be accepted - IMO people sometimes do use it on posix systems while mingw is too exotic case.

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.

4 participants