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: Build libzmq locally to work on all host types #2307

Merged
merged 11 commits into from
Apr 6, 2024

Conversation

pabera
Copy link
Collaborator

@pabera pabera commented Mar 25, 2024

This PR addresses and aims to resolve the building issues associated with libzmq across all platforms.

Previously, the strategy of utilizing pre-built libzmq binaries within Docker environments was intended to streamline development processes. However, this approach did not achieve the desired outcome of creating a seamless and frustration-free development experience.

We are shifting back to locally building libzmq. This process is now consolidated within a dedicated Dockerfile, ensuring that the build is required only once. Once the build is completed, it becomes readily accessible and shareable across all Docker environments, significantly improving the development workflow.

Introduces changes

  • Outsourced libzmq build into local Docker build. Artefacts are shared between Dockerfiles
  • Changed naming convention for Docker files: [name].Dockerfile to Dockerfile.[name] (easier to find in file tree)
  • Updated documentation. Made it easier to copy/paste individual commands by separating them

Should solve:

Relates to:

Test

  • Mac
  • Windows (with WSL)
  • Linux

@pabera pabera self-assigned this Mar 25, 2024
@pabera pabera added the future3 Relates to future3 development label Mar 25, 2024
@pabera pabera added this to the v3.6 milestone Mar 25, 2024
@pabera pabera changed the title Future3/docker-libzmq Fix: Build libzmq locally to work on all host types Mar 25, 2024
Copy link
Collaborator Author

@pabera pabera left a comment

Choose a reason for hiding this comment

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

I have only tested this on Mac so far. If others could help trying this out on their machine, that would be great.

@pabera pabera mentioned this pull request Mar 25, 2024
@pabera pabera changed the title Fix: Build libzmq locally to work on all host types fix: Build libzmq locally to work on all host types Mar 25, 2024
docker/Dockerfile.jukebox Outdated Show resolved Hide resolved
docker/Dockerfile.jukebox Outdated Show resolved Hide resolved
docker/Dockerfile.libzmq Outdated Show resolved Hide resolved
docker/Dockerfile.jukebox Outdated Show resolved Hide resolved
@pabera
Copy link
Collaborator Author

pabera commented Mar 25, 2024

Tested this on Windows (WSL, x86_64) successfully.

Co-authored-by: notapirate <notapirate@users.noreply.github.com>
@s-martin
Copy link
Collaborator

@notapirate could you add a comment, with which platform/configuration it works for you?

@pabera pabera requested a review from s-martin March 26, 2024 16:58
@notapirate
Copy link
Contributor

notapirate commented Mar 26, 2024

Sure, I tested this on Linux (Arch Linux, x86_64) successfully.

@pabera pabera mentioned this pull request Mar 26, 2024
@s-martin
Copy link
Collaborator

s-martin commented Mar 26, 2024

On Windows 11 building the images works now, also running :)

Copy link
Contributor

@notapirate notapirate left a comment

Choose a reason for hiding this comment

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

@pabera you may want to double-check changes in docker.md, I think the item numbering got messed up :)

@s-martin
Copy link
Collaborator

@pabera i just merged #2310, if you use the new future3/develop you could remove the markdownlint-disable / restore tags from docker.md, because this is now configured globally.

@s-martin
Copy link
Collaborator

@pabera you may want to double-check changes in docker.md, I think the item numbering got messed up :)

Not necessary, „markdown magic“ renders the numbering correctly.

@s-martin
Copy link
Collaborator

@pabera
in line 379 and 393 are two more <!-- markdownlint tags, could you remove them as well?

@s-martin
Copy link
Collaborator

s-martin commented Mar 30, 2024

After #2312 is merged the error in the CodeQL action should be gone.

@pabera pabera merged commit 27b23cb into MiczFlor:future3/develop Apr 6, 2024
18 checks passed
@pabera pabera deleted the future3/docker-libzmq branch April 6, 2024 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
future3 Relates to future3 development needs testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants