build: Fix intermediate dotnet openCV image#596
build: Fix intermediate dotnet openCV image#596kate-goldenring merged 1 commit intoproject-akri:mainfrom
Conversation
diconico07
requested review from
Britel,
adithyaj,
bfjelds,
jiria,
kate-goldenring and
romoh
as code owners
|
/add-build-dependency-containers-label |
|
👋 Added [build dependency containers] label :)! |
|
👋 Added [build dependency containers] label :)! |
|
/add-same-version-label |
|
👋 Added [same version] label :)! |
|
👋 Added [same version] label :)! |
diconico07
force-pushed
the
fix-opencv-docker
branch
2 times, most recently
from
0087bcb to
2462fb1
Compare
|
It should be helpful : dotnet/dotnet-docker#1537 |
diconico07
force-pushed
the
fix-opencv-docker
branch
from
2462fb1 to
2ebc179
Compare
|
Thanks @jbpaux, this should do the trick now ... And for this one in particular, I don't see much added value in maintaining this image, maybe we can find an alternative to opencv here that doesn't need hours of compilation on our side. |
kate-goldenring
left a comment
kate-goldenring
left a comment
There was a problem hiding this comment.
Just one nit on a missing platform and then LGTM!
| opencv-base-build-amd64: | ||
| ifeq (1, ${BUILD_AMD64}) | ||
| docker build $(CACHE_OPTION) -f $(INTERMEDIATE_DOCKERFILE_DIR)/Dockerfile.opencvsharp-build . -t $(PREFIX)/opencvsharp-build:$(BUILD_OPENCV_BASE_VERSION)-$(AMD64_SUFFIX) --build-arg PLATFORM_TAG=3.1-buster-slim | ||
| docker build $(CACHE_OPTION) -f $(INTERMEDIATE_DOCKERFILE_DIR)/Dockerfile.opencvsharp-build . -t $(PREFIX)/opencvsharp-build:$(BUILD_OPENCV_BASE_VERSION)-$(AMD64_SUFFIX) --build-arg PLATFORM= |
There was a problem hiding this comment.
Looks like platform is missing here
| docker build $(CACHE_OPTION) -f $(INTERMEDIATE_DOCKERFILE_DIR)/Dockerfile.opencvsharp-build . -t $(PREFIX)/opencvsharp-build:$(BUILD_OPENCV_BASE_VERSION)-$(AMD64_SUFFIX) --build-arg PLATFORM= | |
| docker build $(CACHE_OPTION) -f $(INTERMEDIATE_DOCKERFILE_DIR)/Dockerfile.opencvsharp-build . -t $(PREFIX)/opencvsharp-build:$(BUILD_OPENCV_BASE_VERSION)-$(AMD64_SUFFIX) --build-arg PLATFORM=-$(AMD64_SUFFIX) |
There was a problem hiding this comment.
I should add a comment here (and maybe remove the build-arg argument as well) this is intended, aspnet amd64 images are not suffixed and the other uses of the PLATFORM variable expect empty for amd64.
+1 to finding a way not to maintain this opencv image |
|
@diconico07 once this is in, we can then merge #594 and then #593, right? |
|
@kate-goldenring that's right, we will also need another quick PR I didn't submit yet to change the |
The image was built using the wrong version of opencvsharp, fix this. Also switch to shallow clone to reduce build time (a bit). Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com>
diconico07
force-pushed
the
fix-opencv-docker
branch
from
2ebc179 to
f74af6d
Compare
|
Sounds good. @diconico07 lets merge these in after this one: #599 |
3 participants
What this PR does / why we need it:
The image was built using the wrong version of opencvsharp, fix this.
Also switch to shallow clone to reduce build time (a bit).
Should fix #580
Special notes for your reviewer:
If applicable:
cargo fmt)cargo build)cargo clippy)cargo test)cargo doc)