| Message ID | 20211021091007.460641-2-amorenoz@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Facilitate external use of ovn-detrace | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On 10/21/21 11:10 AM, Adrian Moreno wrote: > Currently, if "make" is run after the project is built, the root > manpage (ovn-detrace.1) is rebuilt unnecessarily. > > The reason is that its dependencies are wrong: files such as > lib/common.man or lib/ovs.tmac do not exist in the project's root > path so "make" will constantly rebuild the manpage target. Instead, those > dependencies live in $ovs_src. > > Modify sodepends.py to generate a makefile that contains the variable > names that point the right paths. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > --- Hi Adrian, I confirm this fixes the issue. I have some questions though. > Makefile.am | 2 +- > build-aux/sodepends.py | 45 ++++++++++++++++++++++++++++++++++++++---- > manpages.mk | 12 +++++------ > 3 files changed, 48 insertions(+), 11 deletions(-) > > diff --git a/Makefile.am b/Makefile.am > index 0169c96ef..3b0df8393 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -425,7 +425,7 @@ CLEANFILES += flake8-check > > include $(srcdir)/manpages.mk > $(srcdir)/manpages.mk: $(MAN_ROOTS) build-aux/sodepends.py $(OVS_SRCDIR)/python/build/soutil.py > - @PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python $(PYTHON3) $(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) -I$(OVS_MANDIR) $(MAN_ROOTS) >$(@F).tmp > + @PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python $(PYTHON3) $(srcdir)/build-aux/sodepends.py -I. -Isrcdir,$(srcdir) -IOVS_MANDIR,$(OVS_MANDIR) $(MAN_ROOTS) >$(@F).tmp > @if cmp -s $(@F).tmp $@; then \ > touch $@; \ > rm -f $(@F).tmp; \ > diff --git a/build-aux/sodepends.py b/build-aux/sodepends.py > index 45812bcbd..343fda1af 100755 > --- a/build-aux/sodepends.py > +++ b/build-aux/sodepends.py > @@ -16,9 +16,44 @@ > > from build import soutil > import sys > +import getopt > +import os > > > -def sodepends(include_dirs, filenames, dst): > +def parse_include_dirs(): > + include_dirs = [] > + options, args = getopt.gnu_getopt(sys.argv[1:], 'I:', ['include=']) > + for key, value in options: > + if key in ['-I', '--include']: > + include_dirs.append(value.split(',')) > + else: > + assert False > + > + include_dirs.append(['.']) > + return include_dirs, args Why don't we use soutil.parse_include_dirs() directly and add the 'value.split(,)' there? > + > + > +def find_include_file(include_info, name): > + for info in include_info: > + if len(info) == 2: > + dir = info[1] > + var = "$(%s)/" % info[0] > + else: > + dir = info[0] > + var = "" > + > + file = "%s/%s" % (dir, name) > + try: > + os.stat(file) > + return var + name > + except OSError: > + pass > + sys.stderr.write("%s not found in: %s\n" % > + (name, ' '.join(str(include_info)))) > + return None > + Shouldn't this go to soutil.py in OVS instead? > + > +def sodepends(include_info, filenames, dst): > ok = True > print("# Generated automatically -- do not modify! " > "-*- buffer-read-only: t -*-") > @@ -28,6 +63,7 @@ def sodepends(include_dirs, filenames, dst): > continue > > # Open file. > + include_dirs = [info[0] for info in include_info] > fn = soutil.find_file(include_dirs, toplevel) > if not fn: > ok = False > @@ -47,8 +83,9 @@ def sodepends(include_dirs, filenames, dst): > > name = soutil.extract_include_directive(line) > if name: > - if soutil.find_file(include_dirs, name): > - dependencies.append(name) > + include_file = find_include_file(include_info, name) > + if include_file: > + dependencies.append(include_file) > else: > ok = False > > @@ -62,6 +99,6 @@ def sodepends(include_dirs, filenames, dst): > > > if __name__ == '__main__': > - include_dirs, args = soutil.parse_include_dirs() > + include_dirs, args = parse_include_dirs() > error = not sodepends(include_dirs, args, sys.stdout) > sys.exit(1 if error else 0) +Numan In general, I'm not completely sure about why sodepends.py needs to be part of OVN and why we can't use the OVS version? Regards, Dumitru
On 11/5/21 17:12, Dumitru Ceara wrote: > On 10/21/21 11:10 AM, Adrian Moreno wrote: >> Currently, if "make" is run after the project is built, the root >> manpage (ovn-detrace.1) is rebuilt unnecessarily. >> >> The reason is that its dependencies are wrong: files such as >> lib/common.man or lib/ovs.tmac do not exist in the project's root >> path so "make" will constantly rebuild the manpage target. Instead, those >> dependencies live in $ovs_src. >> >> Modify sodepends.py to generate a makefile that contains the variable >> names that point the right paths. >> >> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >> --- > > Hi Adrian, > > I confirm this fixes the issue. I have some questions though. > >> Makefile.am | 2 +- >> build-aux/sodepends.py | 45 ++++++++++++++++++++++++++++++++++++++---- >> manpages.mk | 12 +++++------ >> 3 files changed, 48 insertions(+), 11 deletions(-) >> >> diff --git a/Makefile.am b/Makefile.am >> index 0169c96ef..3b0df8393 100644 >> --- a/Makefile.am >> +++ b/Makefile.am >> @@ -425,7 +425,7 @@ CLEANFILES += flake8-check >> >> include $(srcdir)/manpages.mk >> $(srcdir)/manpages.mk: $(MAN_ROOTS) build-aux/sodepends.py $(OVS_SRCDIR)/python/build/soutil.py >> - @PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python $(PYTHON3) $(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) -I$(OVS_MANDIR) $(MAN_ROOTS) >$(@F).tmp >> + @PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python $(PYTHON3) $(srcdir)/build-aux/sodepends.py -I. -Isrcdir,$(srcdir) -IOVS_MANDIR,$(OVS_MANDIR) $(MAN_ROOTS) >$(@F).tmp >> @if cmp -s $(@F).tmp $@; then \ >> touch $@; \ >> rm -f $(@F).tmp; \ >> diff --git a/build-aux/sodepends.py b/build-aux/sodepends.py >> index 45812bcbd..343fda1af 100755 >> --- a/build-aux/sodepends.py >> +++ b/build-aux/sodepends.py >> @@ -16,9 +16,44 @@ >> >> from build import soutil >> import sys >> +import getopt >> +import os >> >> >> -def sodepends(include_dirs, filenames, dst): >> +def parse_include_dirs(): >> + include_dirs = [] >> + options, args = getopt.gnu_getopt(sys.argv[1:], 'I:', ['include=']) >> + for key, value in options: >> + if key in ['-I', '--include']: >> + include_dirs.append(value.split(',')) >> + else: >> + assert False >> + >> + include_dirs.append(['.']) >> + return include_dirs, args > > Why don't we use soutil.parse_include_dirs() directly and add the > 'value.split(,)' there? > >> + >> + >> +def find_include_file(include_info, name): >> + for info in include_info: >> + if len(info) == 2: >> + dir = info[1] >> + var = "$(%s)/" % info[0] >> + else: >> + dir = info[0] >> + var = "" >> + >> + file = "%s/%s" % (dir, name) >> + try: >> + os.stat(file) >> + return var + name >> + except OSError: >> + pass >> + sys.stderr.write("%s not found in: %s\n" % >> + (name, ' '.join(str(include_info)))) >> + return None >> + > > Shouldn't this go to soutil.py in OVS instead? > >> + >> +def sodepends(include_info, filenames, dst): >> ok = True >> print("# Generated automatically -- do not modify! " >> "-*- buffer-read-only: t -*-") >> @@ -28,6 +63,7 @@ def sodepends(include_dirs, filenames, dst): >> continue >> >> # Open file. >> + include_dirs = [info[0] for info in include_info] >> fn = soutil.find_file(include_dirs, toplevel) >> if not fn: >> ok = False >> @@ -47,8 +83,9 @@ def sodepends(include_dirs, filenames, dst): >> >> name = soutil.extract_include_directive(line) >> if name: >> - if soutil.find_file(include_dirs, name): >> - dependencies.append(name) >> + include_file = find_include_file(include_info, name) >> + if include_file: >> + dependencies.append(include_file) >> else: >> ok = False >> >> @@ -62,6 +99,6 @@ def sodepends(include_dirs, filenames, dst): >> >> >> if __name__ == '__main__': >> - include_dirs, args = soutil.parse_include_dirs() >> + include_dirs, args = parse_include_dirs() >> error = not sodepends(include_dirs, args, sys.stdout) >> sys.exit(1 if error else 0) > > +Numan > > In general, I'm not completely sure about why sodepends.py needs to be > part of OVN and why we can't use the OVS version? OVS does not have this problem as everything is under the root tree so I assumed it would be preferred to implement it in OVN rather than add stuff to OVS that will not be used by OVS. But I don't have a strong opinion on this. If you prefer to implement this in OVS and use it from OVN, I'll remove this patch from the series, send it to OVS and send a patch to OVN that uses it. > > Regards, > Dumitru > >
On 11/8/21 12:45 PM, Adrian Moreno wrote: > > > On 11/5/21 17:12, Dumitru Ceara wrote: >> On 10/21/21 11:10 AM, Adrian Moreno wrote: >>> Currently, if "make" is run after the project is built, the root >>> manpage (ovn-detrace.1) is rebuilt unnecessarily. >>> >>> The reason is that its dependencies are wrong: files such as >>> lib/common.man or lib/ovs.tmac do not exist in the project's root >>> path so "make" will constantly rebuild the manpage target. Instead, >>> those >>> dependencies live in $ovs_src. >>> >>> Modify sodepends.py to generate a makefile that contains the variable >>> names that point the right paths. >>> >>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >>> --- >> >> Hi Adrian, >> >> I confirm this fixes the issue. I have some questions though. >> >>> Makefile.am | 2 +- >>> build-aux/sodepends.py | 45 ++++++++++++++++++++++++++++++++++++++---- >>> manpages.mk | 12 +++++------ >>> 3 files changed, 48 insertions(+), 11 deletions(-) >>> >>> diff --git a/Makefile.am b/Makefile.am >>> index 0169c96ef..3b0df8393 100644 >>> --- a/Makefile.am >>> +++ b/Makefile.am >>> @@ -425,7 +425,7 @@ CLEANFILES += flake8-check >>> include $(srcdir)/manpages.mk >>> $(srcdir)/manpages.mk: $(MAN_ROOTS) build-aux/sodepends.py >>> $(OVS_SRCDIR)/python/build/soutil.py >>> - >>> @PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python >>> $(PYTHON3) $(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) >>> -I$(OVS_MANDIR) $(MAN_ROOTS) >$(@F).tmp >>> + >>> @PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python >>> $(PYTHON3) $(srcdir)/build-aux/sodepends.py -I. -Isrcdir,$(srcdir) >>> -IOVS_MANDIR,$(OVS_MANDIR) $(MAN_ROOTS) >$(@F).tmp >>> @if cmp -s $(@F).tmp $@; then \ >>> touch $@; \ >>> rm -f $(@F).tmp; \ >>> diff --git a/build-aux/sodepends.py b/build-aux/sodepends.py >>> index 45812bcbd..343fda1af 100755 >>> --- a/build-aux/sodepends.py >>> +++ b/build-aux/sodepends.py >>> @@ -16,9 +16,44 @@ >>> from build import soutil >>> import sys >>> +import getopt >>> +import os >>> -def sodepends(include_dirs, filenames, dst): >>> +def parse_include_dirs(): >>> + include_dirs = [] >>> + options, args = getopt.gnu_getopt(sys.argv[1:], 'I:', ['include=']) >>> + for key, value in options: >>> + if key in ['-I', '--include']: >>> + include_dirs.append(value.split(',')) >>> + else: >>> + assert False >>> + >>> + include_dirs.append(['.']) >>> + return include_dirs, args >> >> Why don't we use soutil.parse_include_dirs() directly and add the >> 'value.split(,)' there? >> >> + >>> + >>> +def find_include_file(include_info, name): >>> + for info in include_info: >>> + if len(info) == 2: >>> + dir = info[1] >>> + var = "$(%s)/" % info[0] >>> + else: >>> + dir = info[0] >>> + var = "" >>> + >>> + file = "%s/%s" % (dir, name) >>> + try: >>> + os.stat(file) >>> + return var + name >>> + except OSError: >>> + pass >>> + sys.stderr.write("%s not found in: %s\n" % >>> + (name, ' '.join(str(include_info)))) >>> + return None >>> + >> >> Shouldn't this go to soutil.py in OVS instead? >> >>> + >>> +def sodepends(include_info, filenames, dst): >>> ok = True >>> print("# Generated automatically -- do not modify! " >>> "-*- buffer-read-only: t -*-") >>> @@ -28,6 +63,7 @@ def sodepends(include_dirs, filenames, dst): >>> continue >>> # Open file. >>> + include_dirs = [info[0] for info in include_info] >>> fn = soutil.find_file(include_dirs, toplevel) >>> if not fn: >>> ok = False >>> @@ -47,8 +83,9 @@ def sodepends(include_dirs, filenames, dst): >>> name = soutil.extract_include_directive(line) >>> if name: >>> - if soutil.find_file(include_dirs, name): >>> - dependencies.append(name) >>> + include_file = find_include_file(include_info, name) >>> + if include_file: >>> + dependencies.append(include_file) >>> else: >>> ok = False >>> @@ -62,6 +99,6 @@ def sodepends(include_dirs, filenames, dst): >>> if __name__ == '__main__': >>> - include_dirs, args = soutil.parse_include_dirs() >>> + include_dirs, args = parse_include_dirs() >>> error = not sodepends(include_dirs, args, sys.stdout) >>> sys.exit(1 if error else 0) >> >> +Numan >> >> In general, I'm not completely sure about why sodepends.py needs to be >> part of OVN and why we can't use the OVS version? > > OVS does not have this problem as everything is under the root tree so I > assumed it would be preferred to implement it in OVN rather than add > stuff to OVS that will not be used by OVS. > But I don't have a strong opinion on this. If you prefer to implement > this in OVS and use it from OVN, I'll remove this patch from the series, > send it to OVS and send a patch to OVN that uses it. > +Mark In any case, this doesn't really block the rest of the series, I think patches 2-5 can be applied already to main branch if maintainers agree. Thanks!
On Mon, Nov 8, 2021 at 6:55 AM Dumitru Ceara <dceara@redhat.com> wrote: > > On 11/8/21 12:45 PM, Adrian Moreno wrote: > > > > > > On 11/5/21 17:12, Dumitru Ceara wrote: > >> On 10/21/21 11:10 AM, Adrian Moreno wrote: > >>> Currently, if "make" is run after the project is built, the root > >>> manpage (ovn-detrace.1) is rebuilt unnecessarily. > >>> > >>> The reason is that its dependencies are wrong: files such as > >>> lib/common.man or lib/ovs.tmac do not exist in the project's root > >>> path so "make" will constantly rebuild the manpage target. Instead, > >>> those > >>> dependencies live in $ovs_src. > >>> > >>> Modify sodepends.py to generate a makefile that contains the variable > >>> names that point the right paths. > >>> > >>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > >>> --- > >> > >> Hi Adrian, > >> > >> I confirm this fixes the issue. I have some questions though. > >> > >>> Makefile.am | 2 +- > >>> build-aux/sodepends.py | 45 ++++++++++++++++++++++++++++++++++++++---- > >>> manpages.mk | 12 +++++------ > >>> 3 files changed, 48 insertions(+), 11 deletions(-) > >>> > >>> diff --git a/Makefile.am b/Makefile.am > >>> index 0169c96ef..3b0df8393 100644 > >>> --- a/Makefile.am > >>> +++ b/Makefile.am > >>> @@ -425,7 +425,7 @@ CLEANFILES += flake8-check > >>> include $(srcdir)/manpages.mk > >>> $(srcdir)/manpages.mk: $(MAN_ROOTS) build-aux/sodepends.py > >>> $(OVS_SRCDIR)/python/build/soutil.py > >>> - > >>> @PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python > >>> $(PYTHON3) $(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) > >>> -I$(OVS_MANDIR) $(MAN_ROOTS) >$(@F).tmp > >>> + > >>> @PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python > >>> $(PYTHON3) $(srcdir)/build-aux/sodepends.py -I. -Isrcdir,$(srcdir) > >>> -IOVS_MANDIR,$(OVS_MANDIR) $(MAN_ROOTS) >$(@F).tmp > >>> @if cmp -s $(@F).tmp $@; then \ > >>> touch $@; \ > >>> rm -f $(@F).tmp; \ > >>> diff --git a/build-aux/sodepends.py b/build-aux/sodepends.py > >>> index 45812bcbd..343fda1af 100755 > >>> --- a/build-aux/sodepends.py > >>> +++ b/build-aux/sodepends.py > >>> @@ -16,9 +16,44 @@ > >>> from build import soutil > >>> import sys > >>> +import getopt > >>> +import os > >>> -def sodepends(include_dirs, filenames, dst): > >>> +def parse_include_dirs(): > >>> + include_dirs = [] > >>> + options, args = getopt.gnu_getopt(sys.argv[1:], 'I:', ['include=']) > >>> + for key, value in options: > >>> + if key in ['-I', '--include']: > >>> + include_dirs.append(value.split(',')) > >>> + else: > >>> + assert False > >>> + > >>> + include_dirs.append(['.']) > >>> + return include_dirs, args > >> > >> Why don't we use soutil.parse_include_dirs() directly and add the > >> 'value.split(,)' there? > >> >> + > >>> + > >>> +def find_include_file(include_info, name): > >>> + for info in include_info: > >>> + if len(info) == 2: > >>> + dir = info[1] > >>> + var = "$(%s)/" % info[0] > >>> + else: > >>> + dir = info[0] > >>> + var = "" > >>> + > >>> + file = "%s/%s" % (dir, name) > >>> + try: > >>> + os.stat(file) > >>> + return var + name > >>> + except OSError: > >>> + pass > >>> + sys.stderr.write("%s not found in: %s\n" % > >>> + (name, ' '.join(str(include_info)))) > >>> + return None > >>> + > >> > >> Shouldn't this go to soutil.py in OVS instead? > >> > >>> + > >>> +def sodepends(include_info, filenames, dst): > >>> ok = True > >>> print("# Generated automatically -- do not modify! " > >>> "-*- buffer-read-only: t -*-") > >>> @@ -28,6 +63,7 @@ def sodepends(include_dirs, filenames, dst): > >>> continue > >>> # Open file. > >>> + include_dirs = [info[0] for info in include_info] > >>> fn = soutil.find_file(include_dirs, toplevel) > >>> if not fn: > >>> ok = False > >>> @@ -47,8 +83,9 @@ def sodepends(include_dirs, filenames, dst): > >>> name = soutil.extract_include_directive(line) > >>> if name: > >>> - if soutil.find_file(include_dirs, name): > >>> - dependencies.append(name) > >>> + include_file = find_include_file(include_info, name) > >>> + if include_file: > >>> + dependencies.append(include_file) > >>> else: > >>> ok = False > >>> @@ -62,6 +99,6 @@ def sodepends(include_dirs, filenames, dst): > >>> if __name__ == '__main__': > >>> - include_dirs, args = soutil.parse_include_dirs() > >>> + include_dirs, args = parse_include_dirs() > >>> error = not sodepends(include_dirs, args, sys.stdout) > >>> sys.exit(1 if error else 0) > >> > >> +Numan > >> > >> In general, I'm not completely sure about why sodepends.py needs to be > >> part of OVN and why we can't use the OVS version? > > > > OVS does not have this problem as everything is under the root tree so I > > assumed it would be preferred to implement it in OVN rather than add > > stuff to OVS that will not be used by OVS. > > But I don't have a strong opinion on this. If you prefer to implement > > this in OVS and use it from OVN, I'll remove this patch from the series, > > send it to OVS and send a patch to OVN that uses it. > > > > +Mark > > In any case, this doesn't really block the rest of the series, I think > patches 2-5 can be applied already to main branch if maintainers agree. Thanks Adrian for this patch series and Dumitru for the reviews. I applied the patches 2-5 to the main branch. For this patch Acked-by: Numan Siddique <numans@ovn.org> My 2 cents - I'm fine modifying the sodepends.py in OVN. I'd personally prefer to move the OVS bits into OVN for man page generation so that we can change the code (in future) if required as per OVN's requirements. Thanks Numan > > Thanks! > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 11/9/21 10:08 PM, Numan Siddique wrote: >>>> +Numan >>>> >>>> In general, I'm not completely sure about why sodepends.py needs to be >>>> part of OVN and why we can't use the OVS version? >>> OVS does not have this problem as everything is under the root tree so I >>> assumed it would be preferred to implement it in OVN rather than add >>> stuff to OVS that will not be used by OVS. >>> But I don't have a strong opinion on this. If you prefer to implement >>> this in OVS and use it from OVN, I'll remove this patch from the series, >>> send it to OVS and send a patch to OVN that uses it. >>> >> +Mark >> >> In any case, this doesn't really block the rest of the series, I think >> patches 2-5 can be applied already to main branch if maintainers agree. > Thanks Adrian for this patch series and Dumitru for the reviews. > > I applied the patches 2-5 to the main branch. > > For this patch > Acked-by: Numan Siddique <numans@ovn.org> > > My 2 cents - I'm fine modifying the sodepends.py in OVN. I'd personally prefer > to move the OVS bits into OVN for man page generation so that we can change > the code (in future) if required as per OVN's requirements. > OK, makes sense to me then. Acked-by: Dumitru Ceara <dceara@redhat.com> Regards, Dumitru
On Thu, Nov 18, 2021 at 9:17 AM Dumitru Ceara <dceara@redhat.com> wrote: > > On 11/9/21 10:08 PM, Numan Siddique wrote: > >>>> +Numan > >>>> > >>>> In general, I'm not completely sure about why sodepends.py needs to be > >>>> part of OVN and why we can't use the OVS version? > >>> OVS does not have this problem as everything is under the root tree so I > >>> assumed it would be preferred to implement it in OVN rather than add > >>> stuff to OVS that will not be used by OVS. > >>> But I don't have a strong opinion on this. If you prefer to implement > >>> this in OVS and use it from OVN, I'll remove this patch from the series, > >>> send it to OVS and send a patch to OVN that uses it. > >>> > >> +Mark > >> > >> In any case, this doesn't really block the rest of the series, I think > >> patches 2-5 can be applied already to main branch if maintainers agree. > > Thanks Adrian for this patch series and Dumitru for the reviews. > > > > I applied the patches 2-5 to the main branch. > > > > For this patch > > Acked-by: Numan Siddique <numans@ovn.org> > > > > My 2 cents - I'm fine modifying the sodepends.py in OVN. I'd personally prefer > > to move the OVS bits into OVN for man page generation so that we can change > > the code (in future) if required as per OVN's requirements. > > > > OK, makes sense to me then. > > Acked-by: Dumitru Ceara <dceara@redhat.com> Thanks. Applied. Numan > > Regards, > Dumitru > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/Makefile.am b/Makefile.am index 0169c96ef..3b0df8393 100644 --- a/Makefile.am +++ b/Makefile.am @@ -425,7 +425,7 @@ CLEANFILES += flake8-check include $(srcdir)/manpages.mk $(srcdir)/manpages.mk: $(MAN_ROOTS) build-aux/sodepends.py $(OVS_SRCDIR)/python/build/soutil.py - @PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python $(PYTHON3) $(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) -I$(OVS_MANDIR) $(MAN_ROOTS) >$(@F).tmp + @PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python $(PYTHON3) $(srcdir)/build-aux/sodepends.py -I. -Isrcdir,$(srcdir) -IOVS_MANDIR,$(OVS_MANDIR) $(MAN_ROOTS) >$(@F).tmp @if cmp -s $(@F).tmp $@; then \ touch $@; \ rm -f $(@F).tmp; \ diff --git a/build-aux/sodepends.py b/build-aux/sodepends.py index 45812bcbd..343fda1af 100755 --- a/build-aux/sodepends.py +++ b/build-aux/sodepends.py @@ -16,9 +16,44 @@ from build import soutil import sys +import getopt +import os -def sodepends(include_dirs, filenames, dst): +def parse_include_dirs(): + include_dirs = [] + options, args = getopt.gnu_getopt(sys.argv[1:], 'I:', ['include=']) + for key, value in options: + if key in ['-I', '--include']: + include_dirs.append(value.split(',')) + else: + assert False + + include_dirs.append(['.']) + return include_dirs, args + + +def find_include_file(include_info, name): + for info in include_info: + if len(info) == 2: + dir = info[1] + var = "$(%s)/" % info[0] + else: + dir = info[0] + var = "" + + file = "%s/%s" % (dir, name) + try: + os.stat(file) + return var + name + except OSError: + pass + sys.stderr.write("%s not found in: %s\n" % + (name, ' '.join(str(include_info)))) + return None + + +def sodepends(include_info, filenames, dst): ok = True print("# Generated automatically -- do not modify! " "-*- buffer-read-only: t -*-") @@ -28,6 +63,7 @@ def sodepends(include_dirs, filenames, dst): continue # Open file. + include_dirs = [info[0] for info in include_info] fn = soutil.find_file(include_dirs, toplevel) if not fn: ok = False @@ -47,8 +83,9 @@ def sodepends(include_dirs, filenames, dst): name = soutil.extract_include_directive(line) if name: - if soutil.find_file(include_dirs, name): - dependencies.append(name) + include_file = find_include_file(include_info, name) + if include_file: + dependencies.append(include_file) else: ok = False @@ -62,6 +99,6 @@ def sodepends(include_dirs, filenames, dst): if __name__ == '__main__': - include_dirs, args = soutil.parse_include_dirs() + include_dirs, args = parse_include_dirs() error = not sodepends(include_dirs, args, sys.stdout) sys.exit(1 if error else 0) diff --git a/manpages.mk b/manpages.mk index 9f7a0ced3..9e3e75fe2 100644 --- a/manpages.mk +++ b/manpages.mk @@ -2,10 +2,10 @@ utilities/ovn-detrace.1: \ utilities/ovn-detrace.1.in \ - lib/common-syn.man \ - lib/common.man \ - lib/ovs.tmac + $(OVS_MANDIR)/lib/common-syn.man \ + $(OVS_MANDIR)/lib/common.man \ + $(OVS_MANDIR)/lib/ovs.tmac utilities/ovn-detrace.1.in: -lib/common-syn.man: -lib/common.man: -lib/ovs.tmac: +$(OVS_MANDIR)/lib/common-syn.man: +$(OVS_MANDIR)/lib/common.man: +$(OVS_MANDIR)/lib/ovs.tmac:
Currently, if "make" is run after the project is built, the root manpage (ovn-detrace.1) is rebuilt unnecessarily. The reason is that its dependencies are wrong: files such as lib/common.man or lib/ovs.tmac do not exist in the project's root path so "make" will constantly rebuild the manpage target. Instead, those dependencies live in $ovs_src. Modify sodepends.py to generate a makefile that contains the variable names that point the right paths. Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- Makefile.am | 2 +- build-aux/sodepends.py | 45 ++++++++++++++++++++++++++++++++++++++---- manpages.mk | 12 +++++------ 3 files changed, 48 insertions(+), 11 deletions(-)