| Message ID | 20220825125217.392555-1-david.marchand@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] github: Skip debian packaging for dpdk-latest. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/intel-ovs-compilation | success | test: success |
Hello, David, On Thu, Aug 25, 2022 at 2:52 PM David Marchand <david.marchand@redhat.com> wrote: > > Debian packaging target builds OVS against a packaged DPDK version. > As a result, it is not relevant in the dpdk-latest branch which follows > DPDK development branch. > > Signed-off-by: David Marchand <david.marchand@redhat.com> > --- > .github/workflows/build-and-test.yml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml > index 58ab85e5d7..b555f45024 100644 > --- a/.github/workflows/build-and-test.yml > +++ b/.github/workflows/build-and-test.yml > @@ -198,6 +198,7 @@ jobs: > path: config.log > > build-linux-deb: > + if: ${{ github.ref != 'refs/heads/dpdk-latest' }} > env: > deb_dependencies: | > linux-headers-$(uname -r) build-essential fakeroot devscripts equivs > -- > 2.37.2 > Thanks for the patch, this makes sense to me! Acked-by: Frode Nordahl <frode.nordahl@canonical.com>
On 8/25/22 14:52, David Marchand wrote: > Debian packaging target builds OVS against a packaged DPDK version. > As a result, it is not relevant in the dpdk-latest branch which follows > DPDK development branch. > > Signed-off-by: David Marchand <david.marchand@redhat.com> > --- > .github/workflows/build-and-test.yml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml > index 58ab85e5d7..b555f45024 100644 > --- a/.github/workflows/build-and-test.yml > +++ b/.github/workflows/build-and-test.yml > @@ -198,6 +198,7 @@ jobs: > path: config.log > > build-linux-deb: > + if: ${{ github.ref != 'refs/heads/dpdk-latest' }} Hmm. This doesn't seem to work for the robot as it doesn't use the 'dpdk-latest' as a branch name. Maybe we should define the DPDK_VER variable globally and check it instead? BTW, not a problem of this patch, but I spotted this: - compiler: gcc dpdk_shared: dpdk_experimental While it should be: - compiler: gcc dpdk_experimental: yes Otherwise, I don't think we're testing experimental APIs. Best regards, Ilya Maximets.
On Wed, Aug 31, 2022 at 2:29 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 8/25/22 14:52, David Marchand wrote: > > Debian packaging target builds OVS against a packaged DPDK version. > > As a result, it is not relevant in the dpdk-latest branch which follows > > DPDK development branch. > > > > Signed-off-by: David Marchand <david.marchand@redhat.com> > > --- > > .github/workflows/build-and-test.yml | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml > > index 58ab85e5d7..b555f45024 100644 > > --- a/.github/workflows/build-and-test.yml > > +++ b/.github/workflows/build-and-test.yml > > @@ -198,6 +198,7 @@ jobs: > > path: config.log > > > > build-linux-deb: > > + if: ${{ github.ref != 'refs/heads/dpdk-latest' }} > > Hmm. This doesn't seem to work for the robot as it doesn't use the > 'dpdk-latest' as a branch name. Maybe we should define the DPDK_VER > variable globally and check it instead? Indeed, filtering on branch name is not enough for patches submitted against dpdk-latest branch. We discussed this offlist. For others, here is a summary of what I tried. I tried to define DPDK_VER as a global env variable. But filtering on this env does not work: I get non-obvious errors from gha yml checker. After reading the documentation, I understand that a job level if can't use global env: context. https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability "jobs.<job_id>.if" is marked as only supporting github, needs, inputs contexts. Funnily, "jobs.<job_id>.steps.if" can use env context, but marking all steps with this is really ugly as a job is started for nothing. Since the "needs" context is supported, I tried to build a pipeline: - added a "dummy" job outputs the global env variable, - then, the debian packaging jobs were made dependent on this dummy job, and the if: keyword used the output of the env variable, This works, but we start a job simply to cover a limitation in GHA. This is ugly. On the other hand, the dpdk-latest already has a patch to select which DPDK version to build against. So when dpdk-latest will be rebased on current master, the simpler is to update this patch and put a if: false in front of the debian packaging jobs as part of the change. > > > BTW, not a problem of this patch, but I spotted this: > > - compiler: gcc > dpdk_shared: dpdk_experimental > > While it should be: > > - compiler: gcc > dpdk_experimental: yes > > Otherwise, I don't think we're testing experimental APIs. Mm, good catch. That is really strange as I remember fixing and testing this stuff... I'll look at it. I think the simpler is to send rebased patches for dpdk-latest. Ian, are you ok with it?
> > On 8/25/22 14:52, David Marchand wrote: > > > Debian packaging target builds OVS against a packaged DPDK version. > > > As a result, it is not relevant in the dpdk-latest branch which follows > > > DPDK development branch. > > > > > > Signed-off-by: David Marchand <david.marchand@redhat.com> > > > --- > > > .github/workflows/build-and-test.yml | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/.github/workflows/build-and-test.yml > b/.github/workflows/build-and-test.yml > > > index 58ab85e5d7..b555f45024 100644 > > > --- a/.github/workflows/build-and-test.yml > > > +++ b/.github/workflows/build-and-test.yml > > > @@ -198,6 +198,7 @@ jobs: > > > path: config.log > > > > > > build-linux-deb: > > > + if: ${{ github.ref != 'refs/heads/dpdk-latest' }} > > > > Hmm. This doesn't seem to work for the robot as it doesn't use the > > 'dpdk-latest' as a branch name. Maybe we should define the DPDK_VER > > variable globally and check it instead? > > Indeed, filtering on branch name is not enough for patches submitted > against dpdk-latest branch. > > We discussed this offlist. > For others, here is a summary of what I tried. > > I tried to define DPDK_VER as a global env variable. > But filtering on this env does not work: I get non-obvious errors from > gha yml checker. > > After reading the documentation, I understand that a job level if > can't use global env: context. > https://docs.github.com/en/actions/learn-github- > actions/contexts#context-availability > "jobs.<job_id>.if" is marked as only supporting github, needs, inputs > contexts. > > Funnily, "jobs.<job_id>.steps.if" can use env context, but marking all > steps with this is really ugly as a job is started for nothing. > > Since the "needs" context is supported, I tried to build a pipeline: > - added a "dummy" job outputs the global env variable, > - then, the debian packaging jobs were made dependent on this dummy > job, and the if: keyword used the output of the env variable, > This works, but we start a job simply to cover a limitation in GHA. > This is ugly. > > On the other hand, the dpdk-latest already has a patch to select which > DPDK version to build against. > So when dpdk-latest will be rebased on current master, the simpler is > to update this patch and put a if: false in front of the debian > packaging jobs as part of the change. > > > > > > > > BTW, not a problem of this patch, but I spotted this: > > > > - compiler: gcc > > dpdk_shared: dpdk_experimental > > > > While it should be: > > > > - compiler: gcc > > dpdk_experimental: yes > > > > Otherwise, I don't think we're testing experimental APIs. > > Mm, good catch. > That is really strange as I remember fixing and testing this stuff... > I'll look at it. > > I think the simpler is to send rebased patches for dpdk-latest. > Ian, are you ok with it? Sure sounds ok to me. Thanks Ian > > > -- > David Marchand
diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 58ab85e5d7..b555f45024 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -198,6 +198,7 @@ jobs: path: config.log build-linux-deb: + if: ${{ github.ref != 'refs/heads/dpdk-latest' }} env: deb_dependencies: | linux-headers-$(uname -r) build-essential fakeroot devscripts equivs
Debian packaging target builds OVS against a packaged DPDK version. As a result, it is not relevant in the dpdk-latest branch which follows DPDK development branch. Signed-off-by: David Marchand <david.marchand@redhat.com> --- .github/workflows/build-and-test.yml | 1 + 1 file changed, 1 insertion(+)