libgdx / Jamepad

A better way to use gamepads in Java
Apache License 2.0
20 stars 13 forks source link

Lower GLIBC requirements by using building linux natives on ubuntu 18.04 docker #23

Closed theofficialgman closed 2 days ago

theofficialgman commented 1 year ago

alternative https://github.com/libgdx/Jamepad/pull/22 closes https://github.com/libgdx/Jamepad/issues/30

streamingdv commented 12 months ago

@theofficialgman hi, just curious does the github action build still work on your side? I had recently problems on my end so I checked out your changes but still it doesn't seem to succeed building the Linux binaries. Here is a test fork of my project

https://github.com/streamingdv/Jamepad/

Are you encountering the same issues in your fork branches? It seems the docker branch fixes the strange issue "Unable to locate package linux-libc-dev:i386" which seems to be occurring recently but fails now at run make -j make not found. E.g.

https://github.com/streamingdv/Jamepad/actions/runs/6625468273/job/17996650912

streamingdv commented 12 months ago

@theofficialgman okay fixed it, I think something has changed in the docker build image

https://github.com/streamingdv/Jamepad/commit/c077df9371fbd876e2e1db47b1e8d8db07013c99

Fails now on the Gradle task...

theofficialgman commented 12 months ago

@theofficialgman okay fixed it, I think something has changed in the docker build image

streamingdv@c077df9

Fails now on the Gradle task...

link:
     [exec] g++: error: /__/__w/Jamepad/Jamepad/SDL/build-linux32/build/.libs/libSDL2.a: No such file or directory

yeah strange. this is happening in jamepad master branch as well. I can reproduce on my x86_64 computer locally as well. there is no libSDL2.a produced by the make command.

this is strange as nothing has changed at all that I can see.

maybe gradle was previously able to use these files instead

│   ├── libSDL2.la
│   ├── libSDL2main.la
│   ├── libSDL2_test.la
streamingdv commented 12 months ago

Yeah I saw that, It's super strange, the make command passes now but the necessary output files weren't build. Nothing changed and I'm not sure what is causing that problem all of a sudden.

I did not yet check on a real machine, does these files maybe get outputted to a different location than previously expected?

streamingdv commented 12 months ago

@theofficialgman for testing purposes I just reverted the build action file to the original and disabled all Linux builds in it (only Windows x64/x86 enabled) and it seems that is working. (I removed the linux-libc-dev:i386 dependency as well)

https://github.com/streamingdv/Jamepad/blob/master/.github/workflows/pushaction.yml

So something different when make is building the Linux stuff. Not sure if it only affects Linux x86 maybe? Windows binaries can be successfully produced

https://github.com/streamingdv/Jamepad/actions/runs/6629530789

theofficialgman commented 11 months ago

ok too me many hours but I finally realized what the actual issue is @streamingdv

[exec] g++: error: /__/__w/Jamepad/Jamepad/SDL/build-linux64/build/.libs/libSDL2.a: No such file or directory

the issue is the /__ the beginning of the filepath.... it was never to do with the SDL2 build itself. the issue lies with gradle

theofficialgman commented 11 months ago

@MrStahlfelge I have spent most of my day trying to bug test your CIs issue with the incorrect filepath issue. This has no relation to this PR as it happens on the master branch as well.

This PR is good for merging, I will leave it up to you to fix your CI

MrStahlfelge commented 11 months ago

Probably the fix is already suggested in #21

theofficialgman commented 11 months ago

Probably the fix is already suggested in #21

Nope. Read again. We have no issues installing dependencies.

Issue is during linking https://github.com/libgdx/Jamepad/pull/23#issuecomment-1783585903

As I said. Fix your scripts as is (they don't work) and then you can come back to merging PRs.

MrStahlfelge commented 11 months ago

Build is fixed now

theofficialgman commented 11 months ago

@MrStahlfelge build failing in same place https://github.com/theofficialgman/Jamepad/actions/runs/6936137715/job/18867810888#step:39:71

streamingdv commented 11 months ago

@theofficialgman I'm still not 100% sure what is causing the issue but I had this problem as well in my fork when I used your action script to build inside a docker container. If you are building the lib normally (without using a container) it will work and you will not run into this problem.

theofficialgman commented 6 months ago

still same linker error https://github.com/theofficialgman/Jamepad/actions/runs/8681588836/job/23804515917#step:39:74

SonicGDX commented 4 months ago

The issue seems to be in jni/build-linux32.xml at line 47 (it's also in build-linux64.xml and the build-windows xmls as well)

<property name="libraries" value="-L/__w/Jamepad/Jamepad/SDL/lib /__/__w/Jamepad/Jamepad/SDL/build-linux32/build/.libs/libSDL2.a -lm -ldl -lpthread"/>

I don't know how/where these xmls are generated but I'm assuming that would be the cause.

SonicGDX commented 4 months ago

I think I found the cause. It seems to be the replaceAll operations in the build.gradle. You can verify this by printing result of getSdl2StaticLibs without and with the replaceall.absolutePath and comparing the outputs.

For Linux x32, for example:

println getSdl2StaticLibs('./SDL/build-linux32/') :

-L/__w/Jamepad/Jamepad/SDL/lib /__w/Jamepad/Jamepad/SDL/lib/libSDL2.a -lm -ldl -lpthread

println getSdl2StaticLibs('./SDL/build-linux32/').replaceAll("[a-zA-Z0-9\\.\\-/]+libSDL2.a", file("SDL/build-linux32/build/.libs/libSDL2.a").absolutePath) :

-L/__w/Jamepad/Jamepad/SDL/lib /__/__w/Jamepad/Jamepad/SDL/build-linux32/build/.libs/libSDL2.a -lm -ldl -lpthread

SonicGDX commented 4 months ago

Looks like the issue is that the underscore character is missing from the regex and therefore is not being matched and replaced. I'll create an issue for this.

SonicGDX commented 4 days ago

@MrStahlfelge

Sorry for the ping. But, if you have free time it would be great if you could review this.

The equivalent PR in the main libGDX repo (https://github.com/libgdx/libgdx/pull/7178) had been merged around a year ago. This one consists of those changes (with a few additional packages installed that I'm assuming were necessary), plus a fix to the regex issue mentioned here (https://github.com/libgdx/Jamepad/issues/30) that was necessary for the build to work, and a few commands also necessary for the build that have been explained above. Finally, the checkout action was downgraded, as it was also necessary to do so for compilation to succeed with the older GLIBC version.

MrStahlfelge commented 2 days ago

Then here we go!

MrStahlfelge commented 2 days ago

Now we need someone to fix the failure :)

theofficialgman commented 2 days ago

just an fyi this will probably fail because of the nodejs 20 forcing that is effect. you have to provide an environment variable to continue using nodejs 16

SonicGDX commented 2 days ago

just an fyi this will probably fail because of the nodejs 20 forcing that is effect. you have to provide an environment variable to continue using nodejs 16

Even with checkout v3? Interesting.

theofficialgman commented 2 days ago

yes because github transitioned to forcing node20 for all actions for anything that previously used node16

SonicGDX commented 2 days ago

yes because github transitioned to forcing node20 for all actions for anything that previously used node16

hmm, seems they might have removed node 16 from the runner entirely?

https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/

theofficialgman commented 2 days ago
ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION=true

https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/

SonicGDX commented 2 days ago
ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION=true

https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/

It says this though.

This will only work until we upgrade the runner removing Node16 later in the spring.

theofficialgman commented 2 days ago

It says this though.

This will only work until we upgrade the runner removing Node16 later in the spring.

Its still not been removed at this point and I do not thing it will ever be removed (for existing images).

Anyway I said this for a reason https://github.com/libgdx/Jamepad/pull/23#discussion_r1799703640

theofficialgman commented 2 days ago

I've been using it in other projects for daily CI and yet to have an instance where node16 wasn't available. Same goes for other projects. I think that node12 is even still available with the same flag (if an actions requests it).

https://github.com/Pi-Apps-Coders/box64-debs/blob/49b7664bdac080f646edff932174d412c87c0b61/.github/workflows/Update-Box64.yml#L10-L12

SonicGDX commented 2 days ago

Hmm, maybe it would suffice then.

Perhaps we should set these 3 environment variables to force it to use 16.

https://github.com/It4innovations/hyperqueue/pull/721/files

ACTIONS_RUNNER_FORCED_INTERNAL_NODE_VERSION: node16 ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION: node16 ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true

theofficialgman commented 2 days ago

The one I sent is all you need.

SonicGDX commented 2 days ago

The one I sent is all you need.

True.

But I was wondering why this isn't an issue in the main libGDX repo.

It turns out they're using checkout@v2.

Tested it on my fork, and it seems to not show any errors.

So it looks like that might be another solution (see https://github.com/libgdx/libgdx/pull/7458). Which is a better approach, do you think?

SonicGDX commented 2 days ago

Never mind, I changed it for the macos job only and it worked, so I assumed it was a fix. But it doesn't seem to solve the issue completely as linux job still broke even with using v2. https://github.com/SonicGDX/Jamepad/actions/runs/11369657049/job/31627733740

So we will probably have to end up using the environment variable approach.

MrStahlfelge commented 2 days ago

Does the approach by setting the env variable in the build script work? I would prefer this over configuring it on the repo for documentation reasons.

theofficialgman commented 2 days ago

It needs to be in the yml since it's the actions that need it, not any of the bash scripts.

SonicGDX commented 2 days ago

Does the approach by setting the env variable in the build script work? I would prefer this over configuring it on the repo for documentation reasons.

It would need to be configured for the workflow, but as theofficialgman says this can be done in the .yml file like this instead of in the repository settings.

I'll make a PR to demonstrate.

SonicGDX commented 2 days ago

Which do you think is better, setting the environment variable across the entire workflow, or setting it only for the linux job (macos job doesn't need it)? I've verified that both work.

The upside of the former is according to GitHub actions warnings, technically actions/setup-java@v3, gradle/gradle-build-action@v2.4.2, and actions/upload-artifact@v3 also use node 16 by default, so setting the environment variable, while it's not required for the macos job could be better for compatibility (without the environment variable, those actions are also forced to use node 20 when they might not have been intended to).

Alternatively, those other actions could probably be updated to versions that work with node 20 without causing any issues for the macos job. But setting the environment variable across the whole workflow is probably easiest. For the Linux job, those older versions would have to be used as the newer versions with node 20 wouldn't work.

macos Node.js 16 actions are deprecated. Please update the following actions to use Node.js 20: actions/setup-java@v3, gradle/gradle-build-action@v2.4.2, actions/upload-artifact@v3. For more information see: https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/.

SonicGDX commented 2 days ago

Uh oh. This deprecation will cause issues after November 30 2024: https://github.blog/changelog/2024-04-16-deprecation-notice-v3-of-the-artifact-actions/

upload-artifact@v3 will stop working on that date. upload-artifact@v4 however uses node 20 so will break this. Even with the environment variable enabled. As shown here.

I'm sure this will be an issue for the main libGDX repo as well. Not sure how this could be solved. It's exactly like you were saying @theofficialgman ,it really is a game.

theofficialgman commented 2 days ago

yeah it is. You can run containers per step but it gets to be more of a hassle than just running the entire job in a container (like this PR did) because the container isn't "saved" after a step completes

https://docs.github.com/en/actions/writing-workflows/choosing-where-your-workflow-runs/running-jobs-in-a-container

there are other people asking for functionality that would allow for runinng a sequence of steps to occur in a container while allowing some steps before/after to occur on the host but github has never implemented it https://github.com/actions/runner/issues/320#issuecomment-582854054 . This is one of the areas where GitLab has been better for a long time https://docs.gitlab.com/ee/ci/yaml/#pre-and-post

SonicGDX commented 2 days ago

Found a potential workaround: https://github.com/actions/checkout/issues/1809#issuecomment-2208337511

Symlinking Node 16 to 20.

Problem is I was looking inside the docker container, and the place where node is located is unfortunately set to read only (mounted to the directory __e), so this might not be straightforward.

SonicGDX commented 2 days ago

yeah it is. You can run containers per step but it gets to be more of a hassle than just running the entire job in a container (like this PR did) because the container isn't "saved" after a step completes

https://docs.github.com/en/actions/writing-workflows/choosing-where-your-workflow-runs/running-jobs-in-a-container

there are other people asking for functionality that would allow for runinng a sequence of steps to occur in a container while allowing some steps before/after to occur on the host but github has never implemented it actions/runner#320 (comment) . This is one of the areas where GitLab has been better for a long time https://docs.gitlab.com/ee/ci/yaml/#pre-and-post

Hmm, maybe the best option would be to go into the directory, download the artifacts, then run the docker script in the working directory. Then after the container exits out, the action runner should have access to that directory and it can upload the artifacts.

See https://docs.github.com/en/actions/sharing-automations/creating-actions/creating-a-docker-container-action#accessing-files-created-by-a-container-action

With minimal reliance on using actions inside the container, I would imagine if we could get that to work it should not break for a long time.

SonicGDX commented 2 days ago

Apologies for requesting a review without testing the changes right beforehand. Since the last time I tested them was months ago, I wrongly assumed that nothing would have changed since then and did not foresee that GitHub could make breaking changes like this.

This PR (https://github.com/libgdx/Jamepad/pull/32) should fix things for now, but we will need to come up with a better solution before GitHub deprecates upload-artifact@v3 and download-artifact@v3.

MrStahlfelge commented 1 day ago

Looks good so far, but push does not work any more which is not related to this problem

SonicGDX commented 1 day ago

Execution failed for task ':publishMavenJavaPublicationToMavenRepository'. Failed to publish publication 'mavenJava' to repository 'maven' Could not PUT 'https://oss.sonatype.org/content/repositories/snapshots/com/***games/jamepad/jamepad/2.30.0.0-SNAPSHOT/maven-metadata.xml'. Received status code 401 from server: Content access is protected by token

~Do you know what might be causing this?~

~EDIT: NVM I misread, workflow before this PR failed too https://github.com/libgdx/Jamepad/actions/runs/10546641552~

~I think it has something to do with the path. /***games/jamepad seems wrong~

SonicGDX commented 1 day ago

@MrStahlfelge

This is probably related: https://community.sonatype.com/t/401-content-access-is-protected-by-token-authentication-failure-while-performing-maven-release/12741

https://central.sonatype.org/news/20240617_migration_of_accounts/#publishing-with-username-password

If a username and password is attempting to be used to authenticate, that's probably why it is failing. You'll probably need to switch to using a User Token.

MrStahlfelge commented 15 hours ago

@Tom-Ski is that something you can handle?

Tom-Ski commented 11 hours ago

Should be good