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

Fix Dockerfile on arm64 #762

Merged
merged 1 commit into from
Feb 26, 2023
Merged

Fix Dockerfile on arm64 #762

merged 1 commit into from
Feb 26, 2023

Conversation

Whizanth
Copy link
Contributor

@Whizanth Whizanth commented Jan 6, 2023

Now that the issue with zippy has been fixed (#756), the only thing preventing Nitter's Dockerfile to successfully build on arm64 is the Nim image.

To solve this, we can use the regular Alpine image, and install Nim manually.

With the changes in this pull request, I was successfully able to build the Nitter image on both arm64 and x86_64.

@unixfox
Copy link
Contributor

unixfox commented Jan 19, 2023

No nim version is enforced in this Dockerfile, I would suggest deploying as a separate Docker image like for invidious: https://github.com/iv-org/invidious/blob/master/docker/Dockerfile.arm64

@Whizanth
Copy link
Contributor Author

I've updated my fork to use a separate Dockerfile for arm64, as you suggested.

As for enforcing the Nim version, as far as I'm able to tell, the latest Nim version that's available in the Alpine repositories is 1.6.8-r0. In contrast, the official Dockerfile for x86 uses 1.6.10, which is newer, so I don't think enforcing an older version would be a good idea, tell me if you don't agree.

If all of this is okay, do I need to squash my commits into one before you merge?

@unixfox
Copy link
Contributor

unixfox commented Jan 28, 2023

As for enforcing the Nim version, as far as I'm able to tell, the latest Nim version that's available in the Alpine repositories is 1.6.8-r0. In contrast, the official Dockerfile for x86 uses 1.6.10, which is newer, so I don't think enforcing an older version would be a good idea, tell me if you don't agree.

That doesn't really matter, it's to avoid using a newer version if one day alpine edge have a much newer version than the version specified in the official Dockerfile.

@zedeus
Copy link
Owner

zedeus commented Feb 25, 2023

Thanks, this looks good. Assuming Docker can automatically use this file to build an ARM image, this should also be reverted: 2aa07e7

@unixfox
Copy link
Contributor

unixfox commented Feb 26, 2023

Thanks, this looks good. Assuming Docker can automatically use this file to build an ARM image, this should also be reverted: 2aa07e7

Here it's a different Dockerfile, it can't be built with the original Dockerfile image.

I think @Whizanth you will need to add a github workflow for pushing the image too.

Dockerfile.arm64 Outdated Show resolved Hide resolved
Dockerfile.arm64 Outdated Show resolved Hide resolved
Dockerfile.arm64 Outdated Show resolved Hide resolved
Copy link
Contributor

@unixfox unixfox left a comment

Choose a reason for hiding this comment

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

Please see my comments

@zedeus zedeus merged commit f4c20a4 into zedeus:master Feb 26, 2023
@zedeus
Copy link
Owner

zedeus commented Feb 26, 2023

Thank you!

@Whizanth
Copy link
Contributor Author

You're welcome! The GitHub workflow will still have to be added though, but I'm not familiar with it.

jackyzy823 pushed a commit to jackyzy823/nitter that referenced this pull request Mar 5, 2023
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.

None yet

3 participants