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.

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

Extract bench according to wiki instructions #4627

Merged
merged 1 commit into from
Jul 3, 2023
Merged

Extract bench according to wiki instructions #4627

merged 1 commit into from
Jul 3, 2023

Conversation

ppigazzini
Copy link
Contributor

@ppigazzini ppigazzini commented Jun 16, 2023

  • loop through the commits starting from the latest one
  • read the bench value from the last match, if any, of the template
    in the commit body text

closes #4627

No functional change

@ppigazzini
Copy link
Contributor Author

In CI it is common for the process to stop immediately when an error occurs. Therefore, using shell: bash {0}, may not be the best approach.

We could make the behavior of posix and msys2 shells consistent, this will enable a nice simplification of the break statement in the loop.

diff --git a/.github/workflows/stockfish_test.yml b/.github/workflows/stockfish_test.yml
index 28218402..708875d1 100644
--- a/.github/workflows/stockfish_test.yml
+++ b/.github/workflows/stockfish_test.yml
@@ -18,38 +18,38 @@ jobs:
             comp: gcc
             run_32bit_tests: true
             run_64bit_tests: true
-            shell: bash {0}
+            shell: bash
           - name: Ubuntu 20.04 Clang
             os: ubuntu-20.04
             compiler: clang++
             comp: clang
             run_32bit_tests: true
             run_64bit_tests: true
-            shell: bash {0}
+            shell: bash
           - name: Android NDK aarch64
             os: ubuntu-22.04
             compiler: aarch64-linux-android21-clang++
             comp: ndk
             run_armv8_tests: true
-            shell: bash {0}
+            shell: bash
           - name: Android NDK arm
             os: ubuntu-22.04
             compiler: armv7a-linux-androideabi21-clang++
             comp: ndk
             run_armv7_tests: true
-            shell: bash {0}
+            shell: bash
           - name: MacOS 12 Apple Clang
             os: macos-12
             compiler: clang++
             comp: clang
             run_64bit_tests: true
-            shell: bash {0}
+            shell: bash
           - name: MacOS 12 GCC 11
             os: macos-12
             compiler: g++-11
             comp: gcc
             run_64bit_tests: true
-            shell: bash {0}
+            shell: bash
           - name: Windows 2022 Mingw-w64 GCC x86_64
             os: windows-2022
             compiler: g++
@@ -115,8 +115,10 @@ jobs:

       - name: Extract the bench number from the commit history
         run: |
-          git log HEAD | grep -o "\b[Bb]ench[ :]\+[1-9][0-9]\{5,9\}\b" | head -n 1 | sed "s/[^0-9]//g" > git_sig
-          [ -s git_sig ] && echo "benchref=$(cat git_sig)" >> $GITHUB_ENV && echo "Reference bench:" $(cat git_sig) || echo "No bench found"
+          for ((n=0; n<100; n++)); do
+            benchref=$(git log HEAD~$n -1 | grep -v '^[[:space:]]*$' | tail -n 1 | grep '\b[Bb]ench[ :]\+[1-9][0-9]\{5,7\}\b' | sed 's/[^0-9]//g') && break || true
+          done
+          [[ -n "$benchref" ]] && echo "benchref=$benchref" >> $GITHUB_ENV && echo "Reference bench: $benchref" || echo "No bench found"

       - name: Check compiler
         run: |

@ppigazzini
Copy link
Contributor Author

ppigazzini commented Jun 16, 2023

Tested successfully the version with shell: bash {0} replaced by shell: bash in all .yml workflow files.
https://github.com/ppigazzini/Stockfish/actions/runs/5294261506/jobs/9583595004

EDIT: PR updated replacing shell: bash {0} with shell: bash to make the posix and msys2 shells consistent.

@ppigazzini
Copy link
Contributor Author

ppigazzini commented Jul 2, 2023

@vondele if your doubts are about the search of the bench value on the last line of the message body, we can easily search for the last line of text that contains the searched pattern, reversing the lines of the message body with tac:

benchref=$(git log HEAD~$n -1 | tac | grep -m 1 -o -x '[[:space:]]*\b[Bb]ench[ :]\+[1-9][0-9]\{5,7\}\b[[:space:]]*' | sed 's/[^0-9]//g')

@vondele
Copy link
Member

vondele commented Jul 2, 2023

yes, I think that's best.

@locutus2
Copy link
Member

locutus2 commented Jul 2, 2023

@vondele if your doubts are about the search of the bench value on the last line of the message body, we can easily search for the last line of text that contains the searched pattern, reversing the lines of the message body with tac:

benchref=$(git log HEAD~$n -1 | tac | grep -m 1 -o -x '[[:space:]]*\b[Bb]ench[ :]\+[1-9][0-9]\{5,7\}\b[[:space:]]*' | sed 's/[^0-9]//g')

I would remove the -x option else this works only if the bench segment is given alone in a single line.

@ppigazzini
Copy link
Contributor Author

ppigazzini commented Jul 2, 2023

I would remove the -x option else this works only if the bench segment is given alone in a single line.

The CI is here to help to write the new commits according to the instruction.
https://github.com/glinscott/fishtest/wiki/Creating-my-first-test#im-ready-for-my-pull-request

Both the commit message and the pull request comment on GitHub must mention if the patch is a 'No functional change' or changes the search. The last line of the commit message should be either 'No functional change' or 'Bench: XXXXXXX' where 'Nodes searched : XXXXXXX' can be found in the output of a ./stockfish bench invocation.

By the way reading the output should help to deal with the ghosts :)

git log HEAD | grep "[Bb]ench" | less

EDIT: we have this nice commit wrote by a devilish person...
This is why I would like to search only on the last line of the message body.

commit 312a248fa9123c48613dcadf2f17c6fd5e0d2a48
Author: <obfuscated>
Date:   Wed Feb 7 01:26:46 2018 +0100

    More robust bench extraction

    Allow travis.yml to recognize a variety of bench formats in commit messages, for instance:

    Bench: 5023593. (really).
    bench: 5023593 (it was 1234567)
    bench : 5023593 (blah blah)
    Bench:5023593
    Bench: 5023593. 567 something (1234567) 563

    No functional change.

@ppigazzini
Copy link
Contributor Author

ppigazzini commented Jul 3, 2023

ready for review.
print the commit sha for debug

From commit: 915532181f11812c80ef0b57bc018de4ea2155ec
Reference bench: 2505168

- loop through the commits starting from the latest one
- read the bench value from the last match, if any, of the template
  in the commit body text

closes #4627

No functional change
@vondele vondele added the to be merged Will be merged shortly label Jul 3, 2023
@vondele vondele closed this in ca5d9a5 Jul 3, 2023
@vondele vondele merged commit ca5d9a5 into official-stockfish:master Jul 3, 2023
22 checks passed
@ppigazzini ppigazzini deleted the github_ci branch July 3, 2023 18:19
Joachim26 pushed a commit to Joachim26/StockfishNPS that referenced this pull request Aug 1, 2023
- loop through the commits starting from the latest one
- read the bench value from the last match, if any, of the template
  in the commit body text

closes official-stockfish#4627

No functional change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
to be merged Will be merged shortly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants