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

Update the dockerfile to fix APCEMM run issue #49

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Calebsakhtar
Copy link
Contributor

@Calebsakhtar Calebsakhtar commented Sep 18, 2024

Issue #47 describes APCEMM hanging when attempting to run the provided example in the docker instance.

@dcdrake pointed out that the vcpkg installation was installing other versions of dependencies than those in the dockerfile, as well as the fact that the wrong version of gcc was being used by the dockerfile. This appears to have been the cause of the above issue.

This PR fixes the above as follows:

  • gcc has been updated to 11.2 in the dockerfile
  • Installation of modules that are also installed by vcpkg have been removed from the dockerfile

The following nice-to-haves have also been done:

  • Updated cmake in the dockerfile to 3.30.3 to avoid the need to download a portable cmake version when compiling APCEMM.
  • Removed the reference to the dockerfile in the APCEMM README when it mentions that the packages are available in the dockerfile.
  • Slight cleanup of the dockerfile.

Thanks a lot @dcdrake!

@Calebsakhtar
Copy link
Contributor Author

@dcdrake Would you mind reviewing this PR?

@dcdrake
Copy link
Contributor

dcdrake commented Sep 18, 2024

Yep I'll check it when I get home

@dcdrake
Copy link
Contributor

dcdrake commented Sep 18, 2024

If the devcontainer gets smaller, and the build still works, that's the best of all worlds.

If you really wanted, you could get rid of the 'zsh' install step at the end and just install bash in the 'apt-get' step. I just liked zsh 🙂.

Looks good!

@Calebsakhtar
Copy link
Contributor Author

@sdeastham I think this PR is ready to be merged after @dcdrake's review!

P.S: To be honest, I also really like zsh so I am choosing to keeping it!

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