| Message ID | 20190425200815.473-1-haleyb.dev@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] OVN: Fix failures after OVS/OVN split | expand |
On Thu, Apr 25, 2019 at 04:08:15PM -0400, Brian Haley wrote: > Added TODO_SPLIT.rst to Makefile.am to avoid an error > during build. > > Removed a section from config-h-check section of Makefile.am > that was no longer relevant. > > Signed-off-by: Brian Haley <haleyb.dev@gmail.com> > --- > Makefile.am | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/Makefile.am b/Makefile.am > index 1b492a314..2bcdb0a76 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -106,6 +106,7 @@ EXTRA_DIST = \ > Vagrantfile-FreeBSD \ > .mailmap \ > TODO.rst \ > + TODO_SPLIT.rst \ > ovn-architecture.7.xml \ > ovn-nb.ovsschema \ > ovn-nb.xml \ > @@ -263,13 +264,6 @@ config-h-check: > echo "See above for list of violations of the rule that"; \ > echo "every C source file must #include <config.h>."; \ > exit 1; \ > - fi; \ > - if grep '#include' include/openvswitch/*.h | \ > - grep -vE '(<.*>)|("openvswitch)|("openflow)'; \ > - then \ > - echo "See above for list of violations of the rule that"; \ > - echo "public openvswitch header file should not include internal library."; \ > - exit 1; \ > fi > .PHONY: config-h-check I wonder whether there should be a similar rule for include/ovn, though? If not: Acked-by: Ben Pfaff <blp@ovn.org>
On 4/25/19 4:26 PM, Ben Pfaff wrote: > On Thu, Apr 25, 2019 at 04:08:15PM -0400, Brian Haley wrote: >> Added TODO_SPLIT.rst to Makefile.am to avoid an error >> during build. >> >> Removed a section from config-h-check section of Makefile.am >> that was no longer relevant. >> >> Signed-off-by: Brian Haley <haleyb.dev@gmail.com> >> --- >> Makefile.am | 8 +------- >> 1 file changed, 1 insertion(+), 7 deletions(-) >> >> diff --git a/Makefile.am b/Makefile.am >> index 1b492a314..2bcdb0a76 100644 >> --- a/Makefile.am >> +++ b/Makefile.am >> @@ -106,6 +106,7 @@ EXTRA_DIST = \ >> Vagrantfile-FreeBSD \ >> .mailmap \ >> TODO.rst \ >> + TODO_SPLIT.rst \ >> ovn-architecture.7.xml \ >> ovn-nb.ovsschema \ >> ovn-nb.xml \ >> @@ -263,13 +264,6 @@ config-h-check: >> echo "See above for list of violations of the rule that"; \ >> echo "every C source file must #include <config.h>."; \ >> exit 1; \ >> - fi; \ >> - if grep '#include' include/openvswitch/*.h | \ >> - grep -vE '(<.*>)|("openvswitch)|("openflow)'; \ >> - then \ >> - echo "See above for list of violations of the rule that"; \ >> - echo "public openvswitch header file should not include internal library."; \ >> - exit 1; \ >> fi >> .PHONY: config-h-check > > I wonder whether there should be a similar rule for include/ovn, though? The top-level include/ only had a single directory, openflow, which itself was empty. I now see there is an ovs/include directory, so maybe that second part needs to change instead of being deleted? It was only generating a non-fatal warning, the TODO_SPLIT.rst was causing a dist-hook-git error. -Brian
On Thu, Apr 25, 2019 at 04:33:03PM -0400, Brian Haley wrote: > On 4/25/19 4:26 PM, Ben Pfaff wrote: > > On Thu, Apr 25, 2019 at 04:08:15PM -0400, Brian Haley wrote: > > > Added TODO_SPLIT.rst to Makefile.am to avoid an error > > > during build. > > > > > > Removed a section from config-h-check section of Makefile.am > > > that was no longer relevant. > > > > > > Signed-off-by: Brian Haley <haleyb.dev@gmail.com> > > > --- > > > Makefile.am | 8 +------- > > > 1 file changed, 1 insertion(+), 7 deletions(-) > > > > > > diff --git a/Makefile.am b/Makefile.am > > > index 1b492a314..2bcdb0a76 100644 > > > --- a/Makefile.am > > > +++ b/Makefile.am > > > @@ -106,6 +106,7 @@ EXTRA_DIST = \ > > > Vagrantfile-FreeBSD \ > > > .mailmap \ > > > TODO.rst \ > > > + TODO_SPLIT.rst \ > > > ovn-architecture.7.xml \ > > > ovn-nb.ovsschema \ > > > ovn-nb.xml \ > > > @@ -263,13 +264,6 @@ config-h-check: > > > echo "See above for list of violations of the rule that"; \ > > > echo "every C source file must #include <config.h>."; \ > > > exit 1; \ > > > - fi; \ > > > - if grep '#include' include/openvswitch/*.h | \ > > > - grep -vE '(<.*>)|("openvswitch)|("openflow)'; \ > > > - then \ > > > - echo "See above for list of violations of the rule that"; \ > > > - echo "public openvswitch header file should not include internal library."; \ > > > - exit 1; \ > > > fi > > > .PHONY: config-h-check > > > > I wonder whether there should be a similar rule for include/ovn, though? > > The top-level include/ only had a single directory, openflow, which itself > was empty. I now see there is an ovs/include directory, so maybe that > second part needs to change instead of being deleted? It was only > generating a non-fatal warning, the TODO_SPLIT.rst was causing a > dist-hook-git error. There might be more than one thing going on here; the principle is just that this is trying to check that public headers don't include non-public headers. Probably Mark has an idea of what should really be done.
Bleep bloop. Greetings Brian Haley, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. git-am: fatal: sha1 information is lacking or useless (Makefile.am). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 OVN: Fix failures after OVS/OVN split The copy of the patch that failed is found in: /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch When you have resolved this problem, run "git am --resolved". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email aconole@bytheb.org Thanks, 0-day Robot
On 4/25/19 4:45 PM, Ben Pfaff wrote: > On Thu, Apr 25, 2019 at 04:33:03PM -0400, Brian Haley wrote: >> On 4/25/19 4:26 PM, Ben Pfaff wrote: >>> On Thu, Apr 25, 2019 at 04:08:15PM -0400, Brian Haley wrote: >>>> Added TODO_SPLIT.rst to Makefile.am to avoid an error >>>> during build. >>>> >>>> Removed a section from config-h-check section of Makefile.am >>>> that was no longer relevant. >>>> >>>> Signed-off-by: Brian Haley <haleyb.dev@gmail.com> >>>> --- >>>> Makefile.am | 8 +------- >>>> 1 file changed, 1 insertion(+), 7 deletions(-) >>>> >>>> diff --git a/Makefile.am b/Makefile.am >>>> index 1b492a314..2bcdb0a76 100644 >>>> --- a/Makefile.am >>>> +++ b/Makefile.am >>>> @@ -106,6 +106,7 @@ EXTRA_DIST = \ >>>> Vagrantfile-FreeBSD \ >>>> .mailmap \ >>>> TODO.rst \ >>>> + TODO_SPLIT.rst \ >>>> ovn-architecture.7.xml \ >>>> ovn-nb.ovsschema \ >>>> ovn-nb.xml \ >>>> @@ -263,13 +264,6 @@ config-h-check: >>>> echo "See above for list of violations of the rule that"; \ >>>> echo "every C source file must #include <config.h>."; \ >>>> exit 1; \ >>>> - fi; \ >>>> - if grep '#include' include/openvswitch/*.h | \ >>>> - grep -vE '(<.*>)|("openvswitch)|("openflow)'; \ >>>> - then \ >>>> - echo "See above for list of violations of the rule that"; \ >>>> - echo "public openvswitch header file should not include internal library."; \ >>>> - exit 1; \ >>>> fi >>>> .PHONY: config-h-check >>> >>> I wonder whether there should be a similar rule for include/ovn, though? >> >> The top-level include/ only had a single directory, openflow, which itself >> was empty. I now see there is an ovs/include directory, so maybe that >> second part needs to change instead of being deleted? It was only >> generating a non-fatal warning, the TODO_SPLIT.rst was causing a >> dist-hook-git error. > > There might be more than one thing going on here; the principle is just > that this is trying to check that public headers don't include > non-public headers. Probably Mark has an idea of what should really be > done. Currently, there's not really a concept of public vs. non-public headers in OVN. Common headers used amongst the different apps are in lib/ . For the time, it's fine to just delete the section. > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Thanks for the correction. I've pushed it to master. On 4/25/19 4:08 PM, Brian Haley wrote: > Added TODO_SPLIT.rst to Makefile.am to avoid an error > during build. > > Removed a section from config-h-check section of Makefile.am > that was no longer relevant. > > Signed-off-by: Brian Haley <haleyb.dev@gmail.com> > --- > Makefile.am | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/Makefile.am b/Makefile.am > index 1b492a314..2bcdb0a76 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -106,6 +106,7 @@ EXTRA_DIST = \ > Vagrantfile-FreeBSD \ > .mailmap \ > TODO.rst \ > + TODO_SPLIT.rst \ > ovn-architecture.7.xml \ > ovn-nb.ovsschema \ > ovn-nb.xml \ > @@ -263,13 +264,6 @@ config-h-check: > echo "See above for list of violations of the rule that"; \ > echo "every C source file must #include <config.h>."; \ > exit 1; \ > - fi; \ > - if grep '#include' include/openvswitch/*.h | \ > - grep -vE '(<.*>)|("openvswitch)|("openflow)'; \ > - then \ > - echo "See above for list of violations of the rule that"; \ > - echo "public openvswitch header file should not include internal library."; \ > - exit 1; \ > fi > .PHONY: config-h-check > >
Hi Mark, Just returning from a break, and I see that we've reached the OVS/OVN separation. Has there been any thought on how to post patches to the mailing list? Will there be a separate mailing list? Any thoughts on how to update the bot? I don't want it to be very spammy. -Aaron 0-day Robot <robot@bytheb.org> writes: > Bleep bloop. Greetings Brian Haley, I am a robot and I have tried out your patch. > Thanks for your contribution. > > I encountered some error that I wasn't expecting. See the details below. > > > git-am: > fatal: sha1 information is lacking or useless (Makefile.am). > Repository lacks necessary blobs to fall back on 3-way merge. > Cannot fall back to three-way merge. > Patch failed at 0001 OVN: Fix failures after OVS/OVN split > The copy of the patch that failed is found in: > /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch > When you have resolved this problem, run "git am --resolved". > If you prefer to skip this patch, run "git am --skip" instead. > To restore the original branch and stop patching, run "git am --abort". > > > Please check this out. If you feel there has been an error, please email aconole@bytheb.org > > Thanks, > 0-day Robot > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Hi Aaron, As of now, there's not a separate mailing list. There's an ongoing discussion in another thread [1] where we are discussing about when to "flip the switch" so to speak on having OVN devs switch over to the new repo for full-time development. With regards to the bot, it's probably best to not have it watch commits to the ovn repo until we switch over to doing full-time development on it. As of right now, the patches that will be going up on it are build system improvements, deletion of irrelevant code, etc. In general, they're not the type of thing that the bot really needs to concern itself with. Mark! [1] https://mail.openvswitch.org/pipermail/ovs-dev/2019-April/358508.html On 4/29/19 1:41 PM, Aaron Conole wrote: > Hi Mark, > > Just returning from a break, and I see that we've reached the OVS/OVN > separation. > > Has there been any thought on how to post patches to the mailing list? > Will there be a separate mailing list? Any thoughts on how to update > the bot? I don't want it to be very spammy. > > -Aaron > > 0-day Robot <robot@bytheb.org> writes: > >> Bleep bloop. Greetings Brian Haley, I am a robot and I have tried out your patch. >> Thanks for your contribution. >> >> I encountered some error that I wasn't expecting. See the details below. >> >> >> git-am: >> fatal: sha1 information is lacking or useless (Makefile.am). >> Repository lacks necessary blobs to fall back on 3-way merge. >> Cannot fall back to three-way merge. >> Patch failed at 0001 OVN: Fix failures after OVS/OVN split >> The copy of the patch that failed is found in: >> /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch >> When you have resolved this problem, run "git am --resolved". >> If you prefer to skip this patch, run "git am --skip" instead. >> To restore the original branch and stop patching, run "git am --abort". >> >> >> Please check this out. If you feel there has been an error, please email aconole@bytheb.org >> >> Thanks, >> 0-day Robot >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Mon, Apr 29, 2019 at 01:59:34PM -0400, Mark Michelson wrote: > With regards to the bot, it's probably best to not have it watch commits to > the ovn repo until we switch over to doing full-time development on it. As > of right now, the patches that will be going up on it are build system > improvements, deletion of irrelevant code, etc. In general, they're not the > type of thing that the bot really needs to concern itself with. I think one of the questions is how the bot should distinguish OVN and OVS patches. This is also an issue for humans. I suggest using [PATCH ovn] in the subject, e.g. --subject-prefix='PATCH ovn' when invoking git send-email.
Ben Pfaff <blp@ovn.org> writes: > On Mon, Apr 29, 2019 at 01:59:34PM -0400, Mark Michelson wrote: >> With regards to the bot, it's probably best to not have it watch commits to >> the ovn repo until we switch over to doing full-time development on it. As >> of right now, the patches that will be going up on it are build system >> improvements, deletion of irrelevant code, etc. In general, they're not the >> type of thing that the bot really needs to concern itself with. > > I think one of the questions is how the bot should distinguish OVN and > OVS patches. This is also an issue for humans. Yes, disambiguation is the difficult part. > I suggest using [PATCH ovn] in the subject, e.g. --subject-prefix='PATCH > ovn' when invoking git send-email. I like this approach, since it will be easy to parse it out and kick off a separate OVN-dedicated bot.
diff --git a/Makefile.am b/Makefile.am index 1b492a314..2bcdb0a76 100644 --- a/Makefile.am +++ b/Makefile.am @@ -106,6 +106,7 @@ EXTRA_DIST = \ Vagrantfile-FreeBSD \ .mailmap \ TODO.rst \ + TODO_SPLIT.rst \ ovn-architecture.7.xml \ ovn-nb.ovsschema \ ovn-nb.xml \ @@ -263,13 +264,6 @@ config-h-check: echo "See above for list of violations of the rule that"; \ echo "every C source file must #include <config.h>."; \ exit 1; \ - fi; \ - if grep '#include' include/openvswitch/*.h | \ - grep -vE '(<.*>)|("openvswitch)|("openflow)'; \ - then \ - echo "See above for list of violations of the rule that"; \ - echo "public openvswitch header file should not include internal library."; \ - exit 1; \ fi .PHONY: config-h-check
Added TODO_SPLIT.rst to Makefile.am to avoid an error during build. Removed a section from config-h-check section of Makefile.am that was no longer relevant. Signed-off-by: Brian Haley <haleyb.dev@gmail.com> --- Makefile.am | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)