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 fixes #230

Merged
merged 5 commits into from
Jun 12, 2023
Merged

MinGW fixes #230

merged 5 commits into from
Jun 12, 2023

Conversation

FtZPetruska
Copy link
Contributor

When working on updating the buildscript to work again on MinGW, I noticed the following issues:

  • Despite passing the correct directory with TOOLCHAIN_DEPS_DIR, pkg-config is not able to find any .pc file.
  • strndup is simply not available.

I addressed the dependency issues the following way:

  • Make the minimum version 3.4, this is what is typically recommended as the bare minimum version to use. This also enables pkg_check_modules to search in prefixes specified in CMAKE_PREFIX_PATH.
  • Make TOOLCHAIN_DEPS_DIR a cache entry. When it is passed through the command-line or via ExternalProject's CMAKE_ARGS, it is set as a cache entry already. Making it a cache entry when not provided avoids weird CMake shadowing issues.
  • Set CMAKE_PREFIX_PATH instead of PKG_CONFIG_PATH. This has the advantage of letting CMake use the correct lib paths for the current system (e.g. when a system uses lib64 instead of lib).

For strndup, I added a wrapper function dupstring that either forwards the call strndup, or use a fallback implementation. The check for whether strndup is available or not is made in CMake, and passed to the wrapper source file only.

- CMake 3.4 is the bare minimum recommended, 3.1 enables pkg-config to
search in CMAKE_PREFIX_PATH.
- TOOLCHAIN_DEPS_DIR should be a cache variable since it's provided by
the commandline.
- Set the CMAKE_PREFIX_PATH instead of PKG_CONFIG_PATH.
This allows building on Windows through Msys.
@FtZPetruska
Copy link
Contributor Author

Another Windows-related issue (which applies to Msys2): vita-headers fails build due to the Makefile generated by vita-libs-gen. The issue comes from the command $(AR) cru $@ $? which fails with the error arm-vita-eabi-ar: Argument list too long when too many object files are used. This is especially an issue with SceLibc as the object file list hits 71620 characters while the limit is around 26k characters.

The workaround for this is to use a response file which contains all the object filenames, and pass it to ar using @file. Since Makefile generated by vita-libs-gen are only meant to be called with arm-vita-eabi-ar, we are guaranteed to have support for this feature.

Fixes #127

src/dupstring.c Outdated Show resolved Hide resolved
@d3m3vilurr
Copy link
Contributor

d3m3vilurr commented Jun 12, 2023

CI issue could be checked by @Princess-of-Sleeping
I'll share to her.


@FtZPetruska hm... changed cmake version may make a trouble. can you cross check it?

@FtZPetruska
Copy link
Contributor Author

CI issue seems to come from:

- uses: actions/checkout@v3

Since psprela is a git submodule, CI should checkout the repo using:

- uses: actions/checkout@v3
  with:
    submodules: true

Checking previous CI runs, it seems that it also was an issue on older CMake versions, but not treated as an error for compatibility:

(run #179)
 CMake Warning (dev) at CMakeLists.txt:11 (add_subdirectory):
  The source directory

    /home/runner/work/vita-toolchain/vita-toolchain/psp2rela

  does not contain a CMakeLists.txt file.

  CMake does not support this case but it used to work accidentally and is
  being allowed for compatibility.

@Princess-of-Sleeping
Copy link
Contributor

Can you also update workflow with this PR?

@FtZPetruska
Copy link
Contributor Author

Can you also update workflow with this PR?

Made the change in 51a2142

@Princess-of-Sleeping
Copy link
Contributor

@d3m3vilurr I think good for now.

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.

3 participants