Skip to content

devel/git: add cross-compilation support#147

Merged
jkloetzke merged 1 commit into
BobBuildTool:masterfrom
mahaase:feature/develgitcross
Jun 20, 2022
Merged

devel/git: add cross-compilation support#147
jkloetzke merged 1 commit into
BobBuildTool:masterfrom
mahaase:feature/develgitcross

Conversation

@mahaase
Copy link
Copy Markdown
Contributor

@mahaase mahaase commented Jun 3, 2022

.FIXES #142

Comment thread recipes/devel/git.yaml Outdated
export LDFLAGS="-L${BOB_DEP_PATHS[libs::zlib-dev]}/usr/lib -L${BOB_DEP_PATHS[libs::openssl-dev]}/usr/lib -Wl,-rpath-link=${BOB_DEP_PATHS[libs::zlib-dev]}/usr/lib -Wl,-rpath-link=${BOB_DEP_PATHS[libs::openssl-dev]}/usr/lib"

export ac_cv_fread_reads_directories=yes
export ac_cv_snprintf_returns_bogus=no
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As I've already mentioned in #142 this depends on the target libc. I don't think it's the proper solution to just hard-code this here. Once someone starts using something other than glibc with the basement this could easily lead to hard-to-debug problems.

Additionally, your changes hard-code these values even for native builds for no good reasons.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hm the second point we can handle easily, so we just do the ac_ stuff for cross-compilation. the dependencies to glibc, mh, no idea: maybe we start with that and if we are on the point for that problem, we have to adjust this?!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[...] maybe we start with that and if we are on the point for that problem, we have to adjust this?!

The problem I see with this approach is that users of the basement might use the devel::git recipe in a recipe stack with their own libc recipe. How are they supposed to know that they have to change the devel::git recipe in order for their setup to work properly? I mean, the compilation would still succeed and all...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In other words: I think we'd have to ensure that compilation of devel::git would fail if you try to compile it with a target libc other than glibc...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would be willing to see if this really ever happens. The current state of the PR only changes the behaviour for cross compiling. This should at least not make things worse. There are other places where we set such variables to hard coded values to get the package cross compiling. So it's not unheard of and so far it worked well enough.

@mahaase mahaase force-pushed the feature/develgitcross branch from 0567723 to 1b3f8b1 Compare June 3, 2022 11:46
@mahaase mahaase force-pushed the feature/develgitcross branch 3 times, most recently from bd22bf8 to 18a7b3d Compare June 16, 2022 12:35
@mahaase
Copy link
Copy Markdown
Contributor Author

mahaase commented Jun 16, 2022

updated.
i did the variables like buildroot:
https://github.com/buildroot/buildroot/blob/master/package/git/git.mk

Copy link
Copy Markdown
Member

@jkloetzke jkloetzke left a comment

Choose a reason for hiding this comment

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

Just some minor thing...

Comment thread recipes/devel/git.yaml Outdated
--without-expat
--without-expat \
--without-iconv \
${EXTRA:+${EXTRA[@]}}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Without double quotes the entries in the array are subject to word splitting. I think this should be prevented:

Suggested change
${EXTRA:+${EXTRA[@]}}
${EXTRA:+"${EXTRA[@]}"}

Comment thread recipes/devel/git.yaml Outdated
export LDFLAGS="-L${BOB_DEP_PATHS[libs::zlib-dev]}/usr/lib -L${BOB_DEP_PATHS[libs::openssl-dev]}/usr/lib -Wl,-rpath-link=${BOB_DEP_PATHS[libs::zlib-dev]}/usr/lib -Wl,-rpath-link=${BOB_DEP_PATHS[libs::openssl-dev]}/usr/lib"

export ac_cv_fread_reads_directories=yes
export ac_cv_snprintf_returns_bogus=no
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would be willing to see if this really ever happens. The current state of the PR only changes the behaviour for cross compiling. This should at least not make things worse. There are other places where we set such variables to hard coded values to get the package cross compiling. So it's not unheard of and so far it worked well enough.

just for cross-compilation:
set some ac_cv_ variables statically in the same way buildroot did too
assume yes for these tests, configure will bail out otherwise
saying error: cannot run test program while cross compiling

.FIXES BobBuildTool#142
@mahaase mahaase force-pushed the feature/develgitcross branch from 18a7b3d to 1451bc9 Compare June 20, 2022 09:53
@jkloetzke jkloetzke merged commit 1f8d898 into BobBuildTool:master Jun 20, 2022
@jkloetzke
Copy link
Copy Markdown
Member

Thanks!

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.

add cross-compilation support for git

3 participants