JeffersonLab / analyzer

HallA C++ Analyzer
BSD 3-Clause "New" or "Revised" License
7 stars 54 forks source link

#188 docker container for halla analyzer #189

Closed panta-123 closed 9 months ago

panta-123 commented 11 months ago

automatic docker container creation for analyzer. Container will be created when a new tag is pushed to github.

we need to add following to gihub secrets.

  1. DOCKERHUB_USERNAME
  2. DOCKERHUB_TOKEN you can get the docken from docker hub.

Checkout the docker image here for validation: https://hub.docker.com/r/apanta123/hallaana Things to be discussed:

export ANALYZER="@CMAKE_INSTALL_PREFIX@"
export PATH="$ANALYZER/@CMAKE_INSTALL_BINDIR@${PATH:+:$PATH}"
export @DY@LD_LIBRARY_PATH="$ANALYZER/@CMAKE_INSTALL_LIBDIR@${@DY@LD_LIBRARY_PATH:+:$@DY@LD_LIBRARY_PATH}"
export ROOT_INCLUDE_PATH="$ANALYZER/@CMAKE_INSTALL_INCLUDEDIR@${ROOT_INCLUDE_PATH:+:$ROOT_INCLUDE_PATH}"
export CMAKE_PREFIX_PATH="$ANALYZER${CMAKE_PREFIX_PATH:+:$CMAKE_PREFIX_PATH}"

currently in docker we have:

ENV PATH="~/local/analyzer/bin:$PATH"
ENV LD_LIBRARY_PATH="~/local/analyzer/lib:$LD_LIBRARY_PATH"
panta-123 commented 11 months ago

@hansenjo , please let me know your thoughts ? we can also have in-person discussion if needed. please try out test docker form here: https://hub.docker.com/r/apanta123/hallaana

hansenjo commented 11 months ago

Thanks for testing this out. I'm in my office (12/C107) this afternoon (Monday, 9-Oct), i.e. right now. Why don't you stop by for a chat?

panta-123 commented 11 months ago

I will wait for test script to be provided and push the changes afte the test. After that we can merge the PR.

And for platform , lets use only amd64 as arm build is not wanting to work.

hansenjo commented 11 months ago

It'll take me a few days to "test the test" thoroughly without making it a kludge. Stay tuned. I am working on it.

panta-123 commented 9 months ago

@ole added the env . Please review the ENV variables set in Dockerfile. And also on run_tests.sh , we need to check if shasum available or not. If not use sha1sum if exists.


ENV CMAKE_INSTALL_PREFIX="/usr/local/analyzer"
ENV LD_LIBRARY_PATH="/usr/local/analyzer/lib64:$LD_LIBRARY_PATH"
ENV ANALYZER="/usr/local/analyzer"
ENV PATH="/usr/local/analyzer/bin:/usr/bin/root:$PATH"
ENV LD_LIBRARY_PATH="/usr/local/analyzer/lib64:$LD_LIBRARY_PATH"
ENV PATH="/${REPO_NAME}-${APP_VERSION}/BUILD/apps:${PATH}"
ENV LD_LIBRARY_PATH="/${REPO_NAME}-${APP_VERSION}/BUILD/HallA:/${REPO_NAME}-${APP_VERSION}/BUILD/Podd:/${REPO_NAME}-${APP_VERSION}/BUILD/hana_decode:/${REPO_NAME}-${APP_VERSION}/BUILD/Database:${LD_LIBRARY_PATH}"
ENV ROOT_INCLUDE_PATH="/${REPO_NAME}-${APP_VERSION}/HallA:/${REPO_NAME}-${APP_VERSION}/Podd:/${REPO_NAME}-${APP_VERSION}/hana_decode:/${REPO_NAME}-${APP_VERSION}/Database:${ROOT_INCLUDE_PATH}"
ENV ROOTSYS="/usr/"

I will provide github action secret via email with instruction on how. This needs to be set before making new tag.

We will see the automated workflow once we make new tag. Lets see how it goes.

hansenjo commented 9 months ago

You do not need to set PATH and LD_LIBRARY_PATH to include the build location. The script already installs the analyzer in ANALYZER=/usr/local/analyzer, so you just need to have $ANALYZER/bin in PATH and $ANALYZER/lib64 in LD_LIBRARY_PATH.

This is true even for running tests/integration/run_tests.sh. The test script will work just fine with an installed analyzer. I only provided the setup_inbuild.[c]sh scripts to avoid having to install every time after a rebuild, but if you already install, the paths set by setup_inbuild aren't needed.

Also, there is no need to set an environment variable CMAKE_INSTALL_PREFIX. Just set an internal CMake variable by passing -DCMAKE_INSTALL_PREFIX=/usr/local/analyzer as an argument at the configuration step, as you already do, and CMake will cache that variable in BUILD/CMakeCache.txt.

panta-123 commented 9 months ago

@hansenjo , I think we are ready to make new tag. Please go ahead. This will be first test so will see how it goes.

hansenjo commented 9 months ago

Merged in commit 170e466eae53965d709eaba8c7311cbfb65f14b8

I pushed a new tag, Release-179. I'll make the corresponding Release in a second.

hansenjo commented 9 months ago

Merged in commit 170e466eae53965d709eaba8c7311cbfb65f14b8