[ofa-general] Re: [PATCH 5/7 V2] osm: QoS - compiling policy file parser

[Sasha Khapyorsky]

On 14:39 Thu 23 Aug     , Yevgeny Kliteynik wrote:
>  Hi Sasha,
> 
>  Sasha Khapyorsky wrote:
> > Yevgeny, Hi again,
> > On 14:53 Tue 21 Aug     , Yevgeny Kliteynik wrote:
> >> Compiling QoS policy file parser.
> >>
> >> C & H files are generated from Lex & Yacc files and the generated files 
> >> are compiled.
> >> Generated files are not included in git, but they do get
> >> included in RPM as they are also created by 'make dist'.
> >>
> >> Signed-off-by: Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
> >> ---
> >>  opensm/include/Makefile.am |    2 ++
> >>  opensm/opensm/Makefile.am  |   16 +++++++++++++++-
> >>  opensm/opensm/configure.in |    2 ++
> >>  3 files changed, 19 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/opensm/include/Makefile.am b/opensm/include/Makefile.am
> >> index ea62507..b83205f 100644
> >> --- a/opensm/include/Makefile.am
> >> +++ b/opensm/include/Makefile.am
> >> @@ -90,6 +90,8 @@ EXTRA_DIST = \
> >>  	$(srcdir)/opensm/osm_state_mgr_ctrl.h \
> >>  	$(srcdir)/opensm/osm_perfmgr.h \
> >>  	$(srcdir)/opensm/osm_perfmgr_db.h \
> >> +	$(srcdir)/opensm/osm_qos_policy.h \
> >> +	$(srcdir)/opensm/osm_qos_parser_y.h \
> >>  	$(srcdir)/complib/cl_thread_osd.h \
> >>  	$(srcdir)/complib/cl_packon.h \
> >>  	$(srcdir)/complib/cl_atomic_osd.h \
> >> diff --git a/opensm/opensm/Makefile.am b/opensm/opensm/Makefile.am
> >> index 6dfa824..31e09f7 100644
> >> --- a/opensm/opensm/Makefile.am
> >> +++ b/opensm/opensm/Makefile.am
> >> @@ -56,7 +56,15 @@ opensm_SOURCES = main.c osm_console.c osm_db_files.c \
> >>  		 osm_ucast_lash.c osm_ucast_file.c osm_ucast_ftree.c \
> >>  		 osm_vl15intf.c osm_vl_arb_rcv.c \
> >>  		 st.c osm_perfmgr.c osm_perfmgr_db.c \
> >> -		 osm_event_plugin.c osm_dump.c
> >> +		 osm_event_plugin.c osm_dump.c \
> >> +		 osm_qos_parser_y.c osm_qos_parser_l.c osm_qos_policy.c
> >> +
> >> +osm_qos_parser_y.c: $(srcdir)/osm_qos_parser.y 
> >> $(srcdir)/../include/opensm/osm_qos_policy.h
> >> +	$(YACC) -y -d $(srcdir)/osm_qos_parser.y -o $(srcdir)/osm_qos_parser_y.c 
> >> --defines=$(srcdir)/../include/opensm/osm_qos_parser_y.h 
> >> --name-prefix=__qos_parser_
> >> +
> >> +osm_qos_parser_l.c: $(srcdir)/osm_qos_parser.l 
> >> $(srcdir)/../include/opensm/osm_qos_policy.h
> >> +	$(LEX) --prefix=__qos_parser_ -o $(srcdir)/osm_qos_parser_l.c 
> >> $(srcdir)/osm_qos_parser.l
> >> +
> > Should generated files be cleaned somewhere?
> 
>  Good question...
>  Should they?

Of course - any generated stuff should be cleaned somehow.

>  I'm not sure. I wouldn't want someone who took the RPM with
>  these files removing the generated files with something like "make clean",
>  and then failing to compile opensm because he doesn't have lex or yacc.

Sure, 'make clean' would be bad choice. But something like
MAINTAINERCLEANFILES?

> 
> >>  if OSMV_OPENIB
> >>  opensm_CFLAGS = -Wall $(OSMV_CFLAGS) -fno-strict-aliasing 
> >> -DVENDOR_RMPP_SUPPORT -DDUAL_SIDED_RMPP $(DBGFLAGS) -D_XOPEN_SOURCE=600 
> >> -D_BSD_SOURCE=1
> >>  opensm_CXXFLAGS = -Wall $(OSMV_CFLAGS) -DVENDOR_RMPP_SUPPORT 
> >> -DDUAL_SIDED_RMPP $(DBGFLAGS) -D_XOPEN_SOURCE=600 -D_BSD_SOURCE=1
> >> @@ -104,3 +112,9 @@ install-exec-hook:
> >>  	if test -L $(DESTDIR)/$(libdir)/libopensm-$(VERSION).so; then rm 
> >> $(DESTDIR)/$(libdir)/libopensm-$(VERSION).so; fi; \
> >>  	lname=`\ls -l $(DESTDIR)/$(libdir)/libopensm.so | awk '{print $$NF}'`; \
> >>  	ln -s $$lname $(DESTDIR)/$(libdir)/libopensm-$(VERSION).so
> >> +
> >> +# generate c and h files from the lex and yacc files
> >> +dist-hook: $(srcdir)/osm_qos_parser.y $(srcdir)/osm_qos_parser.l
> >> +	$(YACC) -y -d $(srcdir)/osm_qos_parser.y -o $(srcdir)/osm_qos_parser_y.c 
> >> --defines=$(srcdir)/../include/opensm/osm_qos_parser_y.h 
> >> --name-prefix=__qos_parser_
> >> +	$(LEX) --prefix=__qos_parser_ -o $(srcdir)/osm_qos_parser_l.c 
> >> $(srcdir)/osm_qos_parser.l
> >> +
> > Will simpler 'dist-hook' like this
> > dist-hook: osm_qos_parser_y.c osm_qos_parser_l.c
> > be fine here?
> 
>  Good idea.
>  I'll repost the patch.
> 
> >> diff --git a/opensm/opensm/configure.in b/opensm/opensm/configure.in
> >> index afb20b3..a49538d 100644
> >> --- a/opensm/opensm/configure.in
> >> +++ b/opensm/opensm/configure.in
> >> @@ -22,6 +22,8 @@ AC_PROG_INSTALL
> >>  AC_PROG_LN_S
> >>  AC_PROG_MAKE_SET
> >>  AC_PROG_LIBTOOL
> >> +AM_PROG_LEX
> >> +AC_PROG_YACC
> > With tarball where *.c files are generated already we will not need lex
> > and yacc (OTOH it is needed for "regular" (or "maintainer-mode") build).
> > Any idea how to avoid this dependencies for tarball builds?
> 
>  I read some stuff on the net, and looks like "configure" doesn't fail if
>  AM_PROG_LEX and AC_PROG_YACC fail to find targets.
>  But I didn't check it yet - didn't have time or machine w/o lex and yacc.

It is risky if it is not "official" feature. I would prefer to get build
failure on my machine due to not installed lex, but not put extra
dependencies for tarball builds. Probably it is better just to remove
this deps.

Sasha