Skip to content

improve docker image script #394

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions platform/windows_x64/BUILD
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
load("@rules_cc//cc:defs.bzl", "cc_toolchain", "cc_toolchain_suite")
load(":windows_cc_toolchain_config.bzl", "cc_toolchain_config")
load("@rules_cc//cc:defs.bzl", "cc_toolchain", "cc_toolchain_suite")

package(default_visibility = ["//visibility:public"])

Expand All @@ -12,7 +12,7 @@ platform(
"@rules_go//go/toolchain:cgo_off",
],
exec_properties = {
"container-image": "docker://645088952840.dkr.ecr.eu-west-1.amazonaws.com/engflow-re/windows-x64:engflow_worker@sha256:6a53c6d5cbc82ca0782e9117482e457a676920227b697de5db90ff53e7afedbe",
"container-image": "<YOUR_WINDOWS_CONTAINER_IMAGE_URI>",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this because of security reasons?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh wait. It looks like we don't run against windows (I think we did before)

Copy link
Contributor

Choose a reason for hiding this comment

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

Please keep this: it's not an example, it's meant to be used in production, even if we're not doing that right now.

"Pool": "windows",
},
)
Expand All @@ -35,13 +35,13 @@ toolchain(

cc_toolchain_suite(
name = "toolchain",
target_compatible_with = [
"@platforms//os:windows",
],
toolchains = {
"x64_windows|msvc": ":msvc_cc_toolchain",
"x64_windows": ":msvc_cc_toolchain",
},
target_compatible_with = [
"@platforms//os:windows",
],
)

cc_toolchain(
Expand Down
80 changes: 41 additions & 39 deletions platform/windows_x64/docker/build.ps1
Copy link
Contributor

Choose a reason for hiding this comment

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

This is based on an internal script docker_build.ps1. Let's keep these in sync. Please update that one first and verify docker build works.

Original file line number Diff line number Diff line change
Expand Up @@ -14,52 +14,54 @@ Set-StrictMode -Version latest
# then invokes 'docker build'.

# Verify that this script was run from the repository root.
if (-not (Test-Path "platform/windows_x64/docker/build.ps1" -Type Leaf) -or -not (Test-Path "WORKSPACE" -Type Leaf)) {
if (-not (Test-Path "platform/windows_x64/docker/build.ps1" -Type Leaf) -or -not (Test-Path "MODULE.bazel" -Type Leaf)) {
Write-Error "Run this script from the repository root directory."
}

# Create a temporary directory and copy files there.
$repoRootDir = Get-Location
Write-Verbose "repoRootDir $repoRootDir"
$buildDir = "${repoRootDir}/_build"
Write-Verbose "Copying files to ${buildDir}..."
New-Item -Type 'directory' -Path $buildDir
trap {
Set-Location $repoRootDir
Remove-Item -Recurse -Force $buildDir
}
Copy-Item 'platform/windows_x64/docker/normalize_os_windows.ps1' $buildDir
Copy-Item 'platform/windows_x64/docker/install_jre.ps1' $buildDir
Copy-Item 'platform/windows_x64/docker/install_msys2.ps1' $buildDir
Copy-Item 'platform/windows_x64/docker/install_python.ps1' $buildDir
Copy-Item 'platform/windows_x64/docker/install_vs_build_tools.ps1' $buildDir
Copy-Item 'platform/windows_x64/docker/Dockerfile' $buildDir

# Build the showincludes wrapper binary.
# TODO(CUS-81): Remove this after
# https://github.com/bazelbuild/bazel/issues/19733 is fixed, and we're upgraded
# to a version of Bazel that has the fix.
bazel build //infra/msvc_filter_showincludes
Copy-Item 'bazel-bin/infra/msvc_filter_showincludes/msvc_filter_showincludes_/msvc_filter_showincludes.exe' $buildDir
try {
Write-Verbose "Copying files to ${buildDir}..."
New-Item -Type 'directory' -Path $buildDir
Copy-Item 'platform/windows_x64/docker/normalize_os_windows.ps1' $buildDir
Copy-Item 'platform/windows_x64/docker/install_jre.ps1' $buildDir
Copy-Item 'platform/windows_x64/docker/install_msys2.ps1' $buildDir
Copy-Item 'platform/windows_x64/docker/install_python.ps1' $buildDir
Copy-Item 'platform/windows_x64/docker/install_vs_build_tools.ps1' $buildDir
Copy-Item 'platform/windows_x64/docker/Dockerfile' $buildDir

# Build the Docker image.
#
# Additional memory is needed to install VS build tools. The default 1GB
# is not enough.
#
# Additional storage space is needed, too: the default 20GB is not enough.
# Configure C:\ProgramData\docker\config\daemon.json with:
#
# "storage-opts": [
# "size=50G"
# ]
Write-Verbose "Building docker image..."
Set-Location $buildDir
docker build `
--tag engflow-container-image `
--memory '4GB' `
.
Set-Location $repoRootDir
# Build the showincludes wrapper binary.
# TODO(CUS-81): Remove this after
# https://github.com/bazelbuild/bazel/issues/19733 is fixed, and we're upgraded
# to a version of Bazel that has the fix.
bazel build //infra/msvc_filter_showincludes
Copy-Item 'bazel-bin/infra/msvc_filter_showincludes/msvc_filter_showincludes_/msvc_filter_showincludes.exe' $buildDir

# Remove the temporary directory on success.
Remove-Item -Recurse -Force $buildDir
# Build the Docker image.
#
# Additional memory is needed to install VS build tools. The default 1GB
# is not enough.
#
# Additional storage space is needed, too: the default 20GB is not enough.
# Configure C:\ProgramData\docker\config\daemon.json with:
#
# "storage-opts": [
# "size=50G"
# ]
Write-Verbose "Building docker image..."
Set-Location $buildDir
docker build `
--tag engflow-container-image `
--memory '4GB' `
.
}
finally {
# Always return to the original directory and clean up
Set-Location $repoRootDir
if (Test-Path $buildDir) {
Remove-Item -Recurse -Force $buildDir
}
}
6 changes: 3 additions & 3 deletions platform/windows_x64/docker/install_vs_build_tools.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ $InformationPreference = "Continue"
$global:ProgressPreference = "SilentlyContinue"

Write-Verbose "Installing VS Buildtools (needed for Bazel)..."
Invoke-WebRequest -Uri "https://storage.googleapis.com/engflow-tools-public/vs_buildtools__859897378.1616711895.exe" -OutFile "c:/vs_buildtools__859897378.1616711895.exe"
Invoke-WebRequest -Uri "https://aka.ms/vs/17/release/vs_buildtools.exe" -OutFile "c:/vs_buildtools.exe"
$installArgs = @(
'--quiet',
'--wait',
Expand All @@ -27,5 +27,5 @@ $installArgs = @(
'--remove',
'Microsoft.VisualStudio.Component.Windows81SDK'
)
Start-Process c:/vs_buildtools__859897378.1616711895.exe -Wait -ArgumentList ($installArgs -join ' ')
Remove-Item "c:/vs_buildtools__859897378.1616711895.exe" -Force
Start-Process c:/vs_buildtools.exe -Wait -ArgumentList ($installArgs -join ' ')
Remove-Item "c:/vs_buildtools.exe" -Force