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

Only initialize the CLI for headless mode and add Dockerfile #2438

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

georgiastuart
Copy link

I've been helping a group use SCIRun on our local cluster. As part of that work, I made a couple changes to SCIRun that may be useful to others. Namely...

  1. Containerization was easier than keeping SCIRun happy with Cluster QT offerings, particularly after a cluster-wide OS update. I created a Dockerfile, which should be broadly useful.
  2. SCIRun refused to start within SLURM batch jobs or CLI interactive jobs and would only run if it was either (a) compiled completely headless or (b) run within our GUI desktop, even when -x was specified, since QT always tried to identify a display. To solve this, I added a check for -x or -h to the Main function to initialize the command line program instead.

Seems to work fine locally. Tested on an Ubuntu 24.04 system using the Apptainer runtime from a docker image.

Georgia Stuart added 5 commits December 2, 2024 08:19
Signed-off-by: Georgia Stuart <georgia.staurt@gmail.com>
Signed-off-by: Georgia Stuart <georgia.staurt@gmail.com>
Signed-off-by: Georgia Stuart <georgia.staurt@gmail.com>
Signed-off-by: Georgia Stuart <georgia.staurt@gmail.com>
Signed-off-by: Georgia Stuart <georgia.staurt@gmail.com>
@georgiastuart
Copy link
Author

I also just pushed a tweak to the docs. Ubuntu 22.04 and later needs different QT5 package names, and I found that I needed to use -DQt_PATH for QT 5 instead of -DQt5_PATH or -DQt5_DIR. I'm happy to modify or revert if this isn't correct across the board, though.

@dcwhite dcwhite self-assigned this Dec 16, 2024
@dcwhite
Copy link
Member

dcwhite commented Dec 16, 2024

Thank you for the Docker file, that was a long-needed feature. I'll check the Windows and Mac builds and then we can probably merge this.

@dcwhite
Copy link
Member

dcwhite commented Dec 16, 2024

Here's the issue, only 7 years old #1720

Signed-off-by: Georgia Stuart <georgia.staurt@gmail.com>
@georgiastuart
Copy link
Author

I think the mac build failed because of the install-qt-action version being out of date. Bumped to v4.

@dcwhite
Copy link
Member

dcwhite commented Dec 16, 2024

I think the mac build failed because of the install-qt-action version being out of date. Bumped to v4.

We might need 4.1 to fix the failure https://github.com/jurplel/install-qt-action/releases/tag/v4.1.0

@georgiastuart
Copy link
Author

Hmm, @v4 should grab the latest in that major version series, so I think it's something else. I'm going to let the rest of the checks finish and then investigate further.

@dcwhite
Copy link
Member

dcwhite commented Dec 16, 2024

According to https://github.com/orgs/community/discussions/48058 and https://github.com/jurplel/install-qt-action/tags, I think in this case it's using the older 4.0.0 tag, and the error looks exactly like 4.1.0's fix. I thought it would grab the latest too, but it seems to be repo-specific. Good to see SCIRun's rusty wheels getting some exercise, at least.

@dcwhite
Copy link
Member

dcwhite commented Dec 16, 2024

While you're updating action versions, would you mind bumping upload-artifact too? https://github.blog/changelog/2024-04-16-deprecation-notice-v3-of-the-artifact-actions/

@georgiastuart
Copy link
Author

georgiastuart commented Dec 17, 2024

I think it is pulling 4.1, that MacOS update refers to this removed brew install, not the pip install: jurplel/install-qt-action@cb07ed2 . I think the v4 label grabs the branch, which is at the 4.1.1 commit.

Looks like it's this and the fix is to let the action manage python, which seems OK since SCIRun builds its own Python: jurplel/install-qt-action#239

Updating that and the request above.

Signed-off-by: Georgia Stuart <georgia.stuart@gmail.com>
@dcwhite
Copy link
Member

dcwhite commented Dec 17, 2024

Hah I hope you're OK with using this PR as a github action cleanup opportunity. The new Mac error is this old line that should not be needed anymore: https://github.com/SCIInstitute/SCIRun/pull/2438/files#diff-38c2966d1144b82978ca2cfe7650879fb15bd6da5845a3c616e8d152b29317f1R90

It was added in 2019 probably to fix some other problem (the commit message gave no details), so it can be deleted. It's in four places in the mac yml file.

src/Main/scirunMain.cc Outdated Show resolved Hide resolved
georgiastuart and others added 2 commits December 17, 2024 19:49
Co-authored-by: Dan White <functoire@gmail.com>
Signed-off-by: Georgia Stuart <georgia.stuart@gmail.com>
@georgiastuart
Copy link
Author

georgiastuart commented Dec 18, 2024

Np on the cicd cleanup opportunity! I removed the symlink lines and (I think) fixed the Windows Qt6 error (the Qt6_PATH env var is no longer set by the install action it looks like).

If y'all have a place to park a Docker image, I can throw in a Docker build workflow. Right now I build and host the container (internally) on our GitLab instance.

- name: Prepare
run: |
ln -s /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/Frameworks/Tk.framework/Versions/8.5/Headers/X11/ /usr/local/include/X11

- name: make
run: ./build.sh -DQt_PATH="${Qt5_Dir}"
Copy link
Member

Choose a reason for hiding this comment

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

Progress! Now the configure is failing, this should be changed to ${Qt_dir} or path or whichever it is now...

@dcwhite
Copy link
Member

dcwhite commented Dec 18, 2024

For now you could try putting something here, I just created it: https://hub.docker.com/repository/docker/functoire/scirun/general
If something is available there, we can get some SCI person to take over the publishing part (I'm no longer employed there).

@dcwhite
Copy link
Member

dcwhite commented Dec 18, 2024

Meanwhile the headless Mac build is failing in Boost code, which would require an upgrade to Boost 1.86. Now that I don't expect you to do, unless you really want to take over as a maintainer...

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.

2 participants