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.

Sign up for GitHub

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 typo for docker.mk #2720

Merged
merged 1 commit into from
Oct 25, 2024
Merged

Fix typo for docker.mk #2720

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Makefile.d/docker.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ docker/name/loadtest:
## build loadtest image
docker/build/loadtest:
@make DOCKERFILE="$(ROOTDIR)/dockers/tools/cli/loadtest/Dockerfile" \
DOCKER_OPTS="$${DOCKER_OPTS:+$${DOCKER_OPTS}} --build-arg ZLIB_VERSION=$(ZLIB_VERSION) --build-arg HDF4_VERSION=$(HDF5_VERSION)" \
DOCKER_OPTS="$${DOCKER_OPTS:+$${DOCKER_OPTS}} --build-arg ZLIB_VERSION=$(ZLIB_VERSION) --build-arg HDF5_VERSION=$(HDF5_VERSION)" \
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Warning: HDF5_VERSION build argument is not used in the Dockerfile

The Dockerfile for the loadtest image does not use the HDF5_VERSION build argument that's being passed in the Makefile. Instead, it installs HDF5 using make hdf5/install without referencing the version argument. This suggests a potential issue where the build argument is being passed but not utilized.

  • dockers/tools/cli/loadtest/Dockerfile: Uses make hdf5/install without version specification
  • Makefile.d/docker.mk:307: Passes unused --build-arg HDF5_VERSION=$(HDF5_VERSION)
🔗 Analysis chain

LGTM! Correct HDF5 version reference for loadtest image.

The build argument now correctly references HDF5_VERSION instead of HDF4_VERSION.

Let's verify the corresponding Dockerfile uses this argument:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the Dockerfile actually uses HDF5_VERSION
rg -l 'ARG HDF5_VERSION' dockers/tools/cli/loadtest/

Length of output: 591

Script:

#!/bin/bash
# Let's try to find the Dockerfile for loadtest and check its content
fd Dockerfile dockers/tools/cli/loadtest/

# Also search for any Dockerfile that uses HDF5_VERSION
fd Dockerfile | xargs rg "ARG HDF5_VERSION"

Length of output: 127

Script:

#!/bin/bash
# Let's examine the content of the loadtest Dockerfile
cat dockers/tools/cli/loadtest/Dockerfile

Length of output: 3562

IMAGE=$(LOADTEST_IMAGE) \
docker/build/image

Expand Down Expand Up @@ -372,7 +372,7 @@ docker/name/benchmark-job:
docker/build/benchmark-job:
@make DOCKERFILE="$(ROOTDIR)/dockers/tools/benchmark/job/Dockerfile" \
IMAGE=$(BENCHMARK_JOB_IMAGE) \
DOCKER_OPTS="$${DOCKER_OPTS:+$${DOCKER_OPTS}} --build-arg ZLIB_VERSION=$(ZLIB_VERSION) --build-arg HDF4_VERSION=$(HDF5_VERSION)" \
DOCKER_OPTS="$${DOCKER_OPTS:+$${DOCKER_OPTS}} --build-arg ZLIB_VERSION=$(ZLIB_VERSION) --build-arg HDF5_VERSION=$(HDF5_VERSION)" \
docker/build/image

.PHONY: docker/name/benchmark-operator
Expand All @@ -395,5 +395,5 @@ docker/name/example-client:
docker/build/example-client:
@make DOCKERFILE="$(ROOTDIR)/dockers/example/client/Dockerfile" \
IMAGE=$(EXAMPLE_CLIENT_IMAGE) \
DOCKER_OPTS="$${DOCKER_OPTS:+$${DOCKER_OPTS}} --build-arg ZLIB_VERSION=$(ZLIB_VERSION) --build-arg HDF4_VERSION=$(HDF5_VERSION)" \
DOCKER_OPTS="$${DOCKER_OPTS:+$${DOCKER_OPTS}} --build-arg ZLIB_VERSION=$(ZLIB_VERSION) --build-arg HDF5_VERSION=$(HDF5_VERSION)" \
docker/build/image