Skip to content

Download mc/minio executables if necessary#93

Merged
bbockelm merged 4 commits intoPelicanPlatform:mainfrom
alexandertuna:AutoDownloadMcMinio
Mar 21, 2025
Merged

Download mc/minio executables if necessary#93
bbockelm merged 4 commits intoPelicanPlatform:mainfrom
alexandertuna:AutoDownloadMcMinio

Conversation

@alexandertuna
Copy link
Collaborator

@alexandertuna alexandertuna commented Mar 14, 2025

A todo list:

  • Add logic to CMakeLists.txt
  • Specify to the tests via an environment variable
  • Add tests?

Test cases I've run:

  • Checking for minio/mc and downloading if they don't exist
    • macos arm64
    • ubuntu arm64
    • ubuntu amd64
  • Running the tests
    • macos arm64, minio/mc already exist
    • macos arm64, minio/mc don't exist

@alexandertuna alexandertuna marked this pull request as ready for review March 15, 2025 00:38
Copy link
Collaborator

@bbockelm bbockelm left a comment

Choose a reason for hiding this comment

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

Existing code looks good.

However, in distro build environments (e.g., RPMs for EPEL), there's no available network connectivity and this would now cause their builds to fail.

I see two options:

  1. Do not make MINIO_BIN and MC_BIN required and allow the unit tests to fail. Downstream packagers will have to hand-label tests to skip.
  2. Add explicit CMake logic that skips tests or activities (such as mc download) that require network connectivity.

@alexandertuna
Copy link
Collaborator Author

Thanks @bbockelm!

I'm confused about the network situation, can you help me understand? Are you saying the build should succeed without minio/mc (in the absence of network) because they're not a compilation dependency, even if the runtime will fail?

@bbockelm bbockelm linked an issue Mar 16, 2025 that may be closed by this pull request
Copy link
Collaborator

@bbockelm bbockelm left a comment

Choose a reason for hiding this comment

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

LGTM!

@bbockelm bbockelm merged commit 4239359 into PelicanPlatform:main Mar 21, 2025
3 checks passed
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.

Automatically download mc and minio for unit tests

2 participants