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

[Sasha Khapyorsky]

On 15:39 Thu 23 Aug     , Yevgeny Kliteynik wrote:
>  Hi Sasha,
> 
>  Sasha Khapyorsky wrote:
> > 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?
> 
>  OK, I'll take care of this - I'll send in in a separate patch later.
> 
> >>>>  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.
> 
>  OK, agree.
>  We will remove them till I have more info on this.
>  Do you want me to repost the patch or you can just remove these lines
>  as you apply it?

I can remove, but anyway this patch should be applied only after 2,3,4.
I think '3' should be improved and content of '2' requires global stuff
which is in '3'... '4' looks fine, but useless without the rest.
So it is up to you - to repost this patch, drop subsequent patch or just
remind me if I will forget to remove :).

Sasha