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

MINGW/MSYS2 : Fetch binary dependencies from the build environnement itself instead of relying on binaries kept in Git #703

Closed
vsonnier opened this issue Jun 29, 2024 · 7 comments
Assignees

Comments

@vsonnier
Copy link
Collaborator

A follow-up of 7cf09be :

We added libwinpthread-1.dll as a binary dependency for MSYS2/MINGW and stored a given version (MSYS2) in /Windows/misc/[x64|x86], in the same way we keep SDL2 and audio codec binaries.

As @j4reporting reported (pun intended) this is not very clean because this dll may have an adherence on the undelaying Mingw/MSYS2 runtime, even built static, so strictly speaking is not "portable" from one build to another.

An improvement would be to fetch 'libwinpthread-1.dll' from the current Mingw distribution used in Makefile.w[32|64] to garantee conststency.

@vsonnier
Copy link
Collaborator Author

@j4reporting Using $MINGW_PREFIX to find the installation looks OK, including for the CI. Is it good on your side ?

@sezero
Copy link
Collaborator

sezero commented Jun 29, 2024

gcc in a mingw-w64 toolchain can be configured to not rely on winpthreads (e.g. my own linux-to-windows mingw-w64 cross-toolchains are as such), therefore, the unconditional copying of it in the makefiles can result in failures.

@j4reporting
Copy link
Contributor

@vsonnier I now realize that the copy commands are in the Makefiles.
This will probably break other mingw installations especially those without posix threads enabled or configured with --disable-shared like w64devkit
It is necessary to check whether this dll exists.

I don't know if this could be moved to the build-mingw.yml workflow file.

@sezero mingw compilers distributed by the msys2 project, w64devkit, or provided by fedora and probably others are using posix threads.

@sezero
Copy link
Collaborator

sezero commented Jun 29, 2024

@sezero mingw compilers distributed by the msys2 project, w64devkit, or provided by fedora and probably others are using posix threads.

That I already know. But what they provide isn't the universal truth, that's what I'm saying.

@j4reporting
Copy link
Contributor

That I already know. But what they provide isn't the universal truth, that's what I'm saying.

I have never claimed that.
I assume this MSYS2 environment is not custom built, but provided by github?
Are there any disadvantages by using posix threads?

My prefered method would be to link the static library instead.
MSYS2 defines $MSYSTEM so just use this specific case for github's mingw workflow
see https://github.com/j4reporting/vkQuake-vso/commit/d79821a392579da612d678eae465a6f609ff25b8
ignore the second part of the commit.

If you want to link the shared library then something like this should work, I would not use MINGW_PREFIX though. Make it specific for MSYS2 for now.

- MINGW_PATH = ${MINGW_PREFIX}
+LIBWINPTHREAD = $(MSYSTEM_PREFIX)/bin/libwinpthread-1.dll


dll:
	if [ -f $(LIBWINPTHREAD) ]; then \
		cp $(LIBWINPTHREAD)  . ; \
	fi

@sezero
Copy link
Collaborator

sezero commented Jun 29, 2024

Are there any disadvantages by using posix threads?

Unnecessary dependence to winpthreads dll itself is a disadvantage, imo.

	if [ -f $(LIBWINPTHREAD) ]; then \
		cp $(LIBWINPTHREAD)  . ; \
	fi

Existence check looks ok to me

@vsonnier
Copy link
Collaborator Author

I've commited @j4reporting suggestion:

If you want to link the shared library then something like this should work, I would not use MINGW_PREFIX though. Make it specific for MSYS2 for now.

- MINGW_PATH = ${MINGW_PREFIX}
+LIBWINPTHREAD = $(MSYSTEM_PREFIX)/bin/libwinpthread-1.dll


dll:
	if [ -f $(LIBWINPTHREAD) ]; then \
		cp $(LIBWINPTHREAD)  . ; \
	fi

For now, this is Good Enough (TM) for me. Feel free to improve later if you feel like it.

Thanks for your help !

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

No branches or pull requests

4 participants