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

Use AX_COMPARE_VERSION macro for golang version checks #987

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

Kangie
Copy link
Contributor

@Kangie Kangie commented Apr 2, 2024

The previous approach fragile, required manual updates, and results in unexpected outcomes when the user has updated golang.

  • What does this PR do?

We stop manually whitelisting golang versions in favour of checking against a minimum version.

I know that meson is coming (yay!), but this keeps biting us in Gentoo. :)

@Kangie Kangie force-pushed the mj/future-proof-golang-detection branch from 1e34faf to 4888a81 Compare April 2, 2024 11:18
@ThomasAdam
Copy link
Member

@Kangie

Thanks -- I was hoping there would be an easier way.

But you've got syntax issues with this change. CI isn't passing. Please fix.

@ThomasAdam ThomasAdam self-assigned this Apr 2, 2024
@ThomasAdam ThomasAdam added the area:build Relates to compiling/buildsystem of fvwm label Apr 2, 2024
@ThomasAdam ThomasAdam added this to the 1.1.1 milestone Apr 2, 2024
@Kangie
Copy link
Contributor Author

Kangie commented Apr 2, 2024

Ah. We can either ship m4/ax-compare-version.m4 or add autoconf-archive to the fvwm3-build container.

Edit: @ThomasAdam I think updating the build container and adding autoconf-archive to the installed packages is the neater solution, but do you have a preference?

@Kangie Kangie force-pushed the mj/future-proof-golang-detection branch 3 times, most recently from af022a1 to e260a6d Compare April 2, 2024 12:28
@ThomasAdam
Copy link
Member

ThomasAdam commented Apr 2, 2024

@Kangie

Thanks. But, please only make the necessary changes required to introduce AX_COMPARE_VERSION -- I am not interested in any other autotools/automake/configure changes at this time.

@Kangie
Copy link
Contributor Author

Kangie commented Apr 2, 2024

Sure, I was hacking away anyway - Some of these changes should simplify your meson port though by removing unnecessary checks.

Do you have a preference around updating the build image or shipping the m4 macro in-repo?

@ThomasAdam
Copy link
Member

Do you have a preference around updating the build image or shipping the m4 macro in-repo?

Add it to acinclude.m4.

@Kangie Kangie force-pushed the mj/future-proof-golang-detection branch from e260a6d to 9be90a9 Compare April 2, 2024 12:50
@Kangie
Copy link
Contributor Author

Kangie commented Apr 2, 2024

Dropped the offending commits.

Please be aware of https://bugs.gentoo.org/919163 (AC_FUNC_SELECT is broken)

Is it worth hacking away at the meson build path instead?

@Kangie Kangie force-pushed the mj/future-proof-golang-detection branch from 9be90a9 to 0bdb24e Compare April 2, 2024 12:55
configure.ac Outdated Show resolved Hide resolved
Copy link
Member

@ThomasAdam ThomasAdam left a comment

Choose a reason for hiding this comment

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

Hi @Kangie

Thanks -- one slight change, please. I've left an inline comment.

The previous approach was fragile, required manual updates,
and results in unexpected outcomes when the user has updated
golang.

Reported-by: Denny Rivetti <[email protected]>
@Kangie Kangie force-pushed the mj/future-proof-golang-detection branch from 0bdb24e to aa58f83 Compare April 2, 2024 21:05
@Kangie
Copy link
Contributor Author

Kangie commented Apr 2, 2024

That should take care of it. I still think that the autotools tidyup I did will make maintaining the code and build system easier going forward, regardless of the system used (i.e. less complexity for meson). Would you consider a separate PR?

@ThomasAdam
Copy link
Member

Hi @Kangie

Thanks. Will merge this.

I'm aware of some of the configure problems, and have some patches myself to address a few of them. For example, the select problem.

I'd much rather effort was put into the meson build...

@ThomasAdam ThomasAdam merged commit 6f4a305 into fvwmorg:main Apr 3, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:build Relates to compiling/buildsystem of fvwm
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants