diff mbox series

[v4,10/21] ci: move the Windows job to the top

Message ID 5bdc6a08a8b8040de3082b1690f16538fcc08682.1548254412.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Offer to run CI/PR builds in Azure Pipelines | expand

Commit Message

Derrick Stolee via GitGitGadget Jan. 23, 2019, 2:40 p.m. UTC
From: Johannes Schindelin <johannes.schindelin@gmx.de>

The Windows job currently takes a whopping ~1h20m to complete. Which is
*far* longer than the next-longest job takes (linux-gcc, ~35m). As such,
it makes sense to start the Windows job first, to minimize the overall
run time (which is now pretty safely the run time of the Windows job).

This affects only the Azure Pipelines configuration, not the Travis one,
of course, as Travis cannot run our Windows job: 1h20m is distinctly
longer than the 50 minute timeout of Travis' free tier.

This commit is best viewed with `--color-moved`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 azure-pipelines.yml | 172 ++++++++++++++++++++++----------------------
 1 file changed, 86 insertions(+), 86 deletions(-)

Comments

Junio C Hamano Jan. 23, 2019, 10:59 p.m. UTC | #1
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The Windows job currently takes a whopping ~1h20m to complete. Which is
> *far* longer than the next-longest job takes (linux-gcc, ~35m). As such,
> it makes sense to start the Windows job first, to minimize the overall
> run time (which is now pretty safely the run time of the Windows job).

Is the reason why Windows job gets started first is to make sure
that it, which is known to take the longest time, never has to wait
before starting while other jobs run, in case there is limited
parallelism?  The last part of this sentence is what readers of this
step will need in order to be convinced by the justification given,
because (1) if the jobs run totally serially, the order does not
matter much---if anything, running shorter jobs first would give
results from more jobs sooner, and (2) if the jobs run totally in
parallel, the order does not matter as long as we have enough
parallelism.

> This commit is best viewed with `--color-moved`.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  azure-pipelines.yml | 172 ++++++++++++++++++++++----------------------
>  1 file changed, 86 insertions(+), 86 deletions(-)

For those who are seeing this azure-pipelines series for the first
time, it would probably be unclear what the point of adding an
entire file in 09/21 and them moving lines around in 10/21 is.  If
somebody asked me why, I wouldn't be able to explain why it is a
good idea.

Would it hurt readability if these two steps are combined?
Junio C Hamano Jan. 23, 2019, 11:07 p.m. UTC | #2
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The Windows job currently takes a whopping ~1h20m to complete. Which is
> *far* longer than the next-longest job takes (linux-gcc, ~35m). As such,
> it makes sense to start the Windows job first, to minimize the overall
> run time (which is now pretty safely the run time of the Windows job).

Is the reason why Windows job gets started first is to make sure
that it, which is known to take the longest time, never has to wait
before starting while other jobs run, in case there is limited
parallelism?  The last part of this sentence is what readers of this
step will need in order to be convinced by the justification given,
because (1) if the jobs run totally serially, the order does not
matter much---if anything, running shorter jobs first would give
results from more jobs sooner, and (2) if the jobs run totally in
parallel, the order does not matter as long as we have enough
parallelism.

> This commit is best viewed with `--color-moved`.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  azure-pipelines.yml | 172 ++++++++++++++++++++++----------------------
>  1 file changed, 86 insertions(+), 86 deletions(-)

For those who are seeing this azure-pipelines series for the first
time, it would probably be unclear what the point of adding an
entire file in 09/21 and them moving lines around in 10/21 is.  If
somebody asked me why, I wouldn't be able to explain why it is a
good idea.

The same comment applies to 11/21.

Would it hurt readability if these steps are combined?

If 09/21 were "copy travis.yml to create a moral-equivalent set-up
for azure.yml", then it is an entirely different story (i.e. "we
start from an equivalent setup as we have, and then tweak to match
our needs better, and we can view the tweak easier as a separate
step"), but I did not get the impression that it was what happened
there in 09/21.
Johannes Schindelin Jan. 27, 2019, 6:22 p.m. UTC | #3
Hi Junio,

On Wed, 23 Jan 2019, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
> 
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > The Windows job currently takes a whopping ~1h20m to complete. Which is
> > *far* longer than the next-longest job takes (linux-gcc, ~35m). As such,
> > it makes sense to start the Windows job first, to minimize the overall
> > run time (which is now pretty safely the run time of the Windows job).
> 
> Is the reason why Windows job gets started first is to make sure
> that it, which is known to take the longest time, never has to wait
> before starting while other jobs run, in case there is limited
> parallelism?

Yes, in order to optimize the overall run time. Like, if you have N jobs,
and you know that one of them takes longer than the other combined, it
really only makes sense to start that one as first one.

> The last part of this sentence is what readers of this
> step will need in order to be convinced by the justification given,
> because (1) if the jobs run totally serially, the order does not
> matter much---if anything, running shorter jobs first would give
> results from more jobs sooner, and (2) if the jobs run totally in
> parallel, the order does not matter as long as we have enough
> parallelism.

Right, I think I totally forgot to mention that Azure Pipelines offers 10
parallel jobs to open source projects for free. Which means that we will
run up to 10 jobs in parallel, as long as no other build is running, of
course.

Will adjust the commit message.

> > This commit is best viewed with `--color-moved`.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  azure-pipelines.yml | 172 ++++++++++++++++++++++----------------------
> >  1 file changed, 86 insertions(+), 86 deletions(-)
> 
> For those who are seeing this azure-pipelines series for the first
> time, it would probably be unclear what the point of adding an
> entire file in 09/21 and them moving lines around in 10/21 is.  If
> somebody asked me why, I wouldn't be able to explain why it is a
> good idea.
> 
> Would it hurt readability if these two steps are combined?

The thing is, I tried (of course) to replicate the Travis configuration as
closely as possible. And the Windows job was in a specific location there.

However, I just realized that I *added* the Windows job to the Pipelines
right from the start, and that is definitely not on par with Travis, as
the Travis configuration did not define a Windows job (instead it has code
to trigger a dedicated Azure Pipeline).

So what I will do instead is to

- *not* add the Windows-specific part in the commit that adds the initial
  Azure Pipelines support, and

- explain in the commit message of that commit that the idea is to imitate
  our existing Travis configuration as closely as possible.

Ciao,
Dscho
Johannes Schindelin Jan. 27, 2019, 7:05 p.m. UTC | #4
Hi Junio,

On Wed, 23 Jan 2019, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
> 
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > The Windows job currently takes a whopping ~1h20m to complete. Which is
> > *far* longer than the next-longest job takes (linux-gcc, ~35m). As such,
> > it makes sense to start the Windows job first, to minimize the overall
> > run time (which is now pretty safely the run time of the Windows job).
> 
> Is the reason why Windows job gets started first is to make sure
> that it, which is known to take the longest time, never has to wait
> before starting while other jobs run, in case there is limited
> parallelism?  The last part of this sentence is what readers of this
> step will need in order to be convinced by the justification given,
> because (1) if the jobs run totally serially, the order does not
> matter much---if anything, running shorter jobs first would give
> results from more jobs sooner, and (2) if the jobs run totally in
> parallel, the order does not matter as long as we have enough
> parallelism.

See my response to the other mail you sent (which seemed to be a first
draft of this mail I am replying to?).

> > This commit is best viewed with `--color-moved`.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  azure-pipelines.yml | 172 ++++++++++++++++++++++----------------------
> >  1 file changed, 86 insertions(+), 86 deletions(-)
> 
> For those who are seeing this azure-pipelines series for the first
> time, it would probably be unclear what the point of adding an
> entire file in 09/21 and them moving lines around in 10/21 is.  If
> somebody asked me why, I wouldn't be able to explain why it is a
> good idea.
> 
> The same comment applies to 11/21.
> 
> Would it hurt readability if these steps are combined?
> 
> If 09/21 were "copy travis.yml to create a moral-equivalent set-up
> for azure.yml", then it is an entirely different story (i.e. "we
> start from an equivalent setup as we have, and then tweak to match
> our needs better, and we can view the tweak easier as a separate
> step"), but I did not get the impression that it was what happened
> there in 09/21.

Indeed, that *was* the intention. I tried to clarify that, by just *not*
adding the Windows-specific part in the commit that adds
azure-pipelines.yml, as it really should imitate .travis.yml closely.

Ciao,
Dscho
diff mbox series

Patch

diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index f3cabb0dd0..e44d2733a4 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -3,6 +3,92 @@  resources:
   fetchDepth: 1
 
 jobs:
+- job: windows
+  displayName: Windows
+  condition: succeeded()
+  pool: Hosted
+  timeoutInMinutes: 240
+  steps:
+  - powershell: |
+      if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") {
+        net use s: \\gitfileshare.file.core.windows.net\test-cache "$GITFILESHAREPWD" /user:AZURE\gitfileshare /persistent:no
+        cmd /c mklink /d "$(Build.SourcesDirectory)\test-cache" S:\
+      }
+    displayName: 'Mount test-cache'
+    env:
+      GITFILESHAREPWD: $(gitfileshare.pwd)
+  - powershell: |
+      # Helper to check the error level of the latest command (exit with error when appropriate)
+      function c() { if (!$?) { exit(1) } }
+
+      # Add build agent's MinGit to PATH
+      $env:PATH = $env:AGENT_HOMEDIRECTORY +"\externals\\git\cmd;" +$env:PATH
+
+      # Helper to initialize (or update) a Git worktree
+      function init ($path, $url, $set_origin) {
+        if (Test-Path $path) {
+          cd $path; c
+          if (Test-Path .git) {
+            & git init; c
+          } else {
+            & git status
+          }
+        } else {
+          & git init $path; c
+          cd $path; c
+        }
+        & git config core.autocrlf false; c
+        & git config core.untrackedCache true; c
+        if (($set_origin -ne 0) -and !(git config remote.origin.url)) {
+          & git remote add origin $url; c
+        }
+        & git fetch --depth=1 $url master; c
+        & git reset --hard FETCH_HEAD; c
+        & git clean -df; c
+      }
+
+      # Initialize Git for Windows' SDK
+      $sdk_path = "$(Build.SourcesDirectory)\git-sdk-64"
+      init "$sdk_path" "https://dev.azure.com/git-for-windows/git-sdk-64/_git/git-sdk-64" 0
+
+      # Let Git ignore the SDK and the test-cache
+      "/git-sdk-64/`n/test-cache/`n" | Out-File -NoNewLine -Encoding ascii -Append "$(Build.SourcesDirectory)\.git\info\exclude"
+    displayName: 'Initialize the Git for Windows SDK'
+  - powershell: |
+      & "git-sdk-64\git-cmd.exe" --command=usr\\bin\\bash.exe -lc @"
+        export MAKEFLAGS=-j10
+        export DEVELOPER=1
+        export NO_PERL=1
+        export NO_SVN_TESTS=1
+        export GIT_TEST_SKIP_REBASE_P=1
+
+        ci/run-build-and-tests.sh || {
+          ci/print-test-failures.sh
+          exit 1
+        }
+      "@
+      if (!$?) { exit(1) }
+    displayName: 'Build & Test'
+    env:
+      HOME: $(Build.SourcesDirectory)
+      MSYSTEM: MINGW64
+  - powershell: |
+      if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") {
+        cmd /c rmdir "$(Build.SourcesDirectory)\test-cache"
+      }
+    displayName: 'Unmount test-cache'
+    condition: true
+    env:
+      GITFILESHAREPWD: $(gitfileshare.pwd)
+  - task: PublishTestResults@2
+    displayName: 'Publish Test Results **/TEST-*.xml'
+    inputs:
+      mergeTestResults: true
+      testRunTitle: 'windows'
+      platform: Windows
+      publishRunAttachments: false
+    condition: succeededOrFailed()
+
 - job: linux_clang
   displayName: linux-clang
   condition: succeeded()
@@ -153,92 +239,6 @@  jobs:
       publishRunAttachments: false
     condition: succeededOrFailed()
 
-- job: windows
-  displayName: Windows
-  condition: succeeded()
-  pool: Hosted
-  timeoutInMinutes: 240
-  steps:
-  - powershell: |
-      if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") {
-        net use s: \\gitfileshare.file.core.windows.net\test-cache "$GITFILESHAREPWD" /user:AZURE\gitfileshare /persistent:no
-        cmd /c mklink /d "$(Build.SourcesDirectory)\test-cache" S:\
-      }
-    displayName: 'Mount test-cache'
-    env:
-      GITFILESHAREPWD: $(gitfileshare.pwd)
-  - powershell: |
-      # Helper to check the error level of the latest command (exit with error when appropriate)
-      function c() { if (!$?) { exit(1) } }
-
-      # Add build agent's MinGit to PATH
-      $env:PATH = $env:AGENT_HOMEDIRECTORY +"\externals\\git\cmd;" +$env:PATH
-
-      # Helper to initialize (or update) a Git worktree
-      function init ($path, $url, $set_origin) {
-        if (Test-Path $path) {
-          cd $path; c
-          if (Test-Path .git) {
-            & git init; c
-          } else {
-            & git status
-          }
-        } else {
-          & git init $path; c
-          cd $path; c
-        }
-        & git config core.autocrlf false; c
-        & git config core.untrackedCache true; c
-        if (($set_origin -ne 0) -and !(git config remote.origin.url)) {
-          & git remote add origin $url; c
-        }
-        & git fetch --depth=1 $url master; c
-        & git reset --hard FETCH_HEAD; c
-        & git clean -df; c
-      }
-
-      # Initialize Git for Windows' SDK
-      $sdk_path = "$(Build.SourcesDirectory)\git-sdk-64"
-      init "$sdk_path" "https://dev.azure.com/git-for-windows/git-sdk-64/_git/git-sdk-64" 0
-
-      # Let Git ignore the SDK and the test-cache
-      "/git-sdk-64/`n/test-cache/`n" | Out-File -NoNewLine -Encoding ascii -Append "$(Build.SourcesDirectory)\.git\info\exclude"
-    displayName: 'Initialize the Git for Windows SDK'
-  - powershell: |
-      & "git-sdk-64\git-cmd.exe" --command=usr\\bin\\bash.exe -lc @"
-        export MAKEFLAGS=-j10
-        export DEVELOPER=1
-        export NO_PERL=1
-        export NO_SVN_TESTS=1
-        export GIT_TEST_SKIP_REBASE_P=1
-
-        ci/run-build-and-tests.sh || {
-          ci/print-test-failures.sh
-          exit 1
-        }
-      "@
-      if (!$?) { exit(1) }
-    displayName: 'Build & Test'
-    env:
-      HOME: $(Build.SourcesDirectory)
-      MSYSTEM: MINGW64
-  - powershell: |
-      if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") {
-        cmd /c rmdir "$(Build.SourcesDirectory)\test-cache"
-      }
-    displayName: 'Unmount test-cache'
-    condition: true
-    env:
-      GITFILESHAREPWD: $(gitfileshare.pwd)
-  - task: PublishTestResults@2
-    displayName: 'Publish Test Results **/TEST-*.xml'
-    inputs:
-      mergeTestResults: true
-      testRunTitle: 'windows'
-      platform: Windows
-      publishRunAttachments: false
-    condition: succeededOrFailed()
-
 - job: linux32
   displayName: Linux32
   condition: succeeded()