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 <[email protected]> >>> --- >> >> 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! _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
