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 official rust image and clean up Dockerfiles #2804

Merged

Conversation

6543
Copy link
Contributor

@6543 6543 commented Apr 13, 2023

  • Prepair buildx to compile for both ARM and x86 architectures.
  • Make the Dockerfile buildable on both ARM and x86 architectures.

Note: Due to briansmith/ring#1414, we cannot cross-compile during the build step. So we either emulate the build step but that will increase build time significant, or we still have to use dedicated agents as it is proposed now

@6543 6543 mentioned this pull request Apr 13, 2023
@6543

This comment was marked as outdated.

@6543 6543 force-pushed the use_official_rust_images_for-arm-support branch from f7f6a23 to ef4c80f Compare April 13, 2023 20:42
@6543 6543 marked this pull request as ready for review April 13, 2023 20:46
@6543 6543 requested review from Nutomic and dessalines as code owners April 13, 2023 20:46
@6543
Copy link
Contributor Author

6543 commented Apr 13, 2023

next test: https://woodpecker.join-lemmy.org/LemmyNet/lemmy/pipeline/119/3

will push a revert commit to the last one to restore the pipeline to what it should alter too ...

@6543

This comment was marked as outdated.

@6543 6543 force-pushed the use_official_rust_images_for-arm-support branch from b245eb2 to 1f5f3a6 Compare April 13, 2023 21:23
@6543 6543 force-pushed the use_official_rust_images_for-arm-support branch from 1f5f3a6 to acc687f Compare April 13, 2023 21:27
@6543 6543 changed the title Use official rust images for arm support WIP: Use official rust images for arm support Apr 14, 2023
@6543

This comment was marked as off-topic.

@6543 6543 mentioned this pull request Apr 14, 2023
@6543 6543 changed the title WIP: Use official rust images for arm support Use official rust image and clean up Dockerfiles Apr 14, 2023
@6543 6543 force-pushed the use_official_rust_images_for-arm-support branch from cf3dd88 to 8bc035a Compare April 14, 2023 01:43
@6543
Copy link
Contributor Author

6543 commented Apr 14, 2023

move cross compile into #2806 as it's blocked by dep "ring" :/

@6543
Copy link
Contributor Author

6543 commented Apr 14, 2023

@dessalines @Nutomic ready to review

Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

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

Wow thank you for doing this!

Is the Dockerfile.debian just for your own setup?

COPY --from=builder /app/lemmy_server /app/lemmy

EXPOSE 8536
CMD ["/app/lemmy"]
Copy link
Member

Choose a reason for hiding this comment

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

Should probably remove this file as cross-compile isnt working. And based on the age of the issue it wont get fixed anytime soon :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This pull does not address cross compilation!

Its moved in a extra pull

Copy link
Member

Choose a reason for hiding this comment

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

Looks like this was added to the main branch anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the multiarch docker image is just ment to be build with buildx ... and the cross-compiline is just an optional feature ... for now I would recomend to just use it as single source of truth and have different agents that target this dockerfile

the old Dockerfile is just for normal devs who wana build the image but dont have buildx installed

Copy link
Member

Choose a reason for hiding this comment

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

In that case I would prefer to remove the old dockerfile, and list buildx as a requirements. cc @dessalines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok fine with that ... I have a look in altering the docs/readme

Copy link
Member

Choose a reason for hiding this comment

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

Yep, I'm def fine with removing the old Dockerfile

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well the "old" is the same as the new one ... just without some fancy if statements and args on FROM 😆. so you can make the multiarch to a normal one again at any time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

docker/Dockerfile.debian Outdated Show resolved Hide resolved
@6543
Copy link
Contributor Author

6543 commented Apr 16, 2023

Deleted :)

@dessalines dessalines merged commit 90c6dc2 into LemmyNet:main Apr 16, 2023
@6543 6543 deleted the use_official_rust_images_for-arm-support branch April 16, 2023 18:17
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.

3 participants