On 30 Nov 2023, at 14:28, Ilya Maximets wrote:
> On 11/30/23 14:25, Ilya Maximets wrote:
>> On 11/30/23 14:08, Eelco Chaudron wrote:
>>>
>>>
>>> On 30 Nov 2023, at 13:29, Simon Horman wrote:
>>>
>>>> On Tue, Nov 28, 2023 at 06:50:59PM +0100, Eelco Chaudron wrote:
>>>>>
>>>>>
>>>>> On 27 Nov 2023, at 19:18, Ilya Maximets wrote:
>>>>>
>>>>>> On 11/27/23 18:36, Eelco Chaudron wrote:
>>>>
>>>> ...
>>>>
>>>>>>> diff --git a/.github/workflows/build-and-test.yml
>>>>>>> b/.github/workflows/build-and-test.yml
>>>>>>> index 09654205e..d15105e7d 100644
>>>>>>> --- a/.github/workflows/build-and-test.yml
>>>>>>> +++ b/.github/workflows/build-and-test.yml
>>>>>>> @@ -223,6 +223,102 @@ jobs:
>>>>>>> name: logs-linux-${{ join(matrix.*, '-') }}
>>>>>>> path: logs.tgz
>>>>>>>
>>>>>>> + build-clang-analyze:
>>>>>>> + needs: build-dpdk
>>>>>>> + env:
>>>>>>> + dependencies: |
>>>>>>> + automake bc clang-tools libbpf-dev libnuma-dev libpcap-dev \
>>>>>>> + libunbound-dev libunwind-dev libssl-dev libtool llvm-dev \
>>>>>>> + python3-unbound
>>>>>>> + CC: clang
>>>>>>> + DPDK: dpdk
>>>>>>> + CLANG_ANALYZE: true
>>>>>>> + name: clang-analyze
>>>>>>> + runs-on: ubuntu-22.04
>>>>>>> + timeout-minutes: 30
>>>>>>> +
>>>>>>> + steps:
>>>>>>> + - name: checkout
>>>>>>> + uses: actions/checkout@v3
>>>>>>> +
>>>>>>> + - name: get base branch sha
>>>>>>> + id: base_branch
>>>>>>> + run: |
>>>>>>> + if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then
>>>>>>> + echo "sha=$BASE_SHA" >> $GITHUB_OUTPUT
>>>>>>> + else
>>>>>>> + if [ "$EVENT_BEFORE" =
>>>>>>> "0000000000000000000000000000000000000000" ]; then
>>>>>>> + echo "sha=$DEFAULT_BRANCH" >> $GITHUB_OUTPUT
>>>>>>
>>>>>> How this is going ot work on patches for older branches?
>>>>>
>>>>> Good question, it’s not :( Took a little bit to figure out how to do
>>>>> this, as we have no reference branch. The solution I came up with was to
>>>>> figure out all parent branches, and use the most recent main/master or
>>>>> branch-x.x one. So it looks like this:
>>>>>
>>>>> if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then
>>>>> echo "sha=$BASE_SHA" >> $GITHUB_OUTPUT
>>>>> else
>>>>> if [ "$EVENT_BEFORE" =
>>>>> "0000000000000000000000000000000000000000" ]; then
>>>>> set sha=$(git log --simplify-by-decoration --decorate=full \
>>>>> --pretty=format:'%d' | \
>>>>> grep -oP 'refs/remotes/origin/\K[^, )]+' | \
>>>>> grep -m1 -E
>>>>> '^master$|^main$|^branch-[0-9]+\.[0-9]+$')
>>>>> [ -z "$sha" ] && echo "sha=$DEFAULT_BRANCH" >> $GITHUB_OUTPUT
>>>>> \
>>>>> || echo "sha=$sha" >> $GITHUB_OUTPUT
>>>>> else
>>>>> echo "sha=$EVENT_BEFORE" >> $GITHUB_OUTPUT
>>>>> fi
>>>>> fi
>>>>
>>>> Hi Eelco,
>>>>
>>>> is this useful here?
>>>>
>>>> git describe --all --no-abbrev --always \
>>>> --match master --match main --match branch-[0-9].[0-9]*
>>>
>>> Thanks Simon, I was looking for something like this, but I was not able to
>>> get it with ‘git describe’, but now I see the match option is the key ;)
>>>
>>> I guess it can now be changed to:
>>>
>>> git describe --all --no-abbrev --always \
>>> --match master --match main --match branch-[0-9].[0-9]* | sed 's/.*\///'
>>>
>>> Will send a v3 next week, waiting for some more potential feedback…
>>
>> Maybe we could just make it simple and test only the last commit (HEAD~1)?
>> Unless the base is explicitly known. It should be enough for a 0-day bot,
>> because it is testing patch sets incrementally, i.e. applying patches one
>> by one.
This will work for the robot, but not on our main branch? But I guess you only
mean for this case where we do not have a valid EVENT_BEFORE?
>> Also, the EVENT_BEFORE seems to not be documented anywhere, I wonder how
>> safe it is to rely on this variable. Or did I miss the docs somewhere?
It’s the API variable ${{ github.event.before }} -->
https://docs.github.com/en/rest/overview/github-event-types?apiVersion=2022-11-28#event-payload-object-for-pushevent
>> Would also be nice to know how this variable behaves on force-push with a
>> branch history rolling back as it might end up with spurious failures during
>> local feature development. I also frequently use the same branches in my
>> fork during code review, force-pushing them frequently, so this process
>> may be affected.
Thought I looked at the forced push as I use this all the time with stg, and I
think it was the ref of the overwritten pushed branch (but I’ll take a look
again before I send the v3).
> One more thing about clang-analyze. Should the clang version be part of
> the cache key? I'm guessing that if the previous cache is constructed
> with an older version of the checker, we may have extra warnings reported,
> unrelated to the current patch.
Good catch!!
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
