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

Fixed some header links, added note about Golang version used. #20

Merged
merged 2 commits into from
Mar 21, 2023

Conversation

maoueh
Copy link
Contributor

@maoueh maoueh commented Jan 10, 2023

On a side note for discussion, I would like to find a way to improve the README to better describe how someone could derive the right value for PKG_CONFIG_PATH and related setup.

In the example, we have:

      - CC=arm-linux-gnueabihf-gcc
      - CXX=arm-linux-gnueabihf-g++
      - CGO_CFLAGS=--sysroot=/sysroot/linux/armhf
      - CGO_LDFLAGS=--sysroot=/sysroot/linux/armhf
      - PKG_CONFIG_SYSROOT_DIR=/sysroot/linux/armhf
      - PKG_CONFIG_PATH=/sysroot/linux/armhf/opt/vc/lib/pkgconfig:/sysroot/linux/armhf/usr/lib/arm-linux-gnueabihf/pkgconfig:/sysroot/linux/armhf/usr/lib/pkgconfig:/sysroot/linux/armhf/usr/local/lib/pkgconfig

While CC and CXX are fairly easy to found, it's harder to know for sure by what armhf needs to be replaced and by what arm-linux-gnueabihf in /sysroot/linux/armhf/usr/lib/arm-linux-gnueabihf is needed.

I think for armhf a quick sentence to tell to replace by Arch column in supported platforms table. For the other one, I would like to add another column Triplet in the the same table. A sentence would then explain that arm-linux-gnueabihf should be replaced in the triplet.

I was wondering also if we should just list the required values per platforms directly instead, does are fairly static and won't change often, so seems it could be a good improvements to add the direct values.

@maoueh
Copy link
Contributor Author

maoueh commented Jan 10, 2023

And for Windows, I'm not even sure what is the required elements, trying now.

@maoueh
Copy link
Contributor Author

maoueh commented Jan 10, 2023

Some of my difficulties might come from the fact that those fields are valid only if an actual sysroot with extra libraries is required. In which case, the creator of the sysroot might know what to put.

@troian
Copy link
Member

troian commented Jan 13, 2023

I know this whole thing with PKG_CONFIG_PATH is way more confusing than we would like it to be, however there is not much that could be improved from our side.

The example you're referring to shows how one can cross-compile go package with C/C++ dependencies for multiple platform at once which is indeed entire purpose of this project.

  1. Remember, PKG_CONFIG_PATH is optional, and only used when dynamic linking is required with C/C++ libraries. For example here we use goreleaser-cross, but there is no mention of PKG_CONFIG_PATH bc C/C++ dependencies (libusb/libhib) actually included in codebase.
  2. armhf historically has been an alias to ARM v7, same as arm64 is an alias to armv8 and amd64 is nearly an alias of x86_64. armhf is widely used within PPA sources for linux distributions. Thats why it was showed this way in the example so users that familiar with c/c++ compiling find thing familiar. For others thats new subject regardless.

p.s: One can name sysroot whatever way it is suitable, as it has nothing to do with actual cross-compiling and it will work as long as directory structure is preserved and libraries are compiled for desired architecture.

@troian troian force-pushed the master branch 15 times, most recently from 73f55c6 to 903176d Compare February 7, 2023 12:07
@maoueh
Copy link
Contributor Author

maoueh commented Mar 15, 2023

@troian Thanks for the extra details, indeed I realized that it was optional after I got the point that those were needed to provide those "external native dependencies" when compiling.

The PR is still good however, fixes some links and add extra details about Golang version used to compile.

@troian
Copy link
Member

troian commented Mar 17, 2023

thanks @maoueh, merging

@troian troian merged commit c6707d6 into goreleaser:master Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants