[ovs-dev] [PATCH] automake: Cleanup to allow make distcheck to succeed.

Aaron Conole aaron at bytheb.org
Sat Jul 18 21:05:54 UTC 2015


Ben Pfaff <blp at nicira.com> writes:

> Can you explain the following change?  What effect does it have, and why
> is it needed only in one fork of the "if"?
>
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -44,7 +44,7 @@ endif
>>  if WIN32
>>  psep=";"
>>  else
>> -psep=":"
>> +psep=:
>>  endif

Make does not accept quoted strings for interpretation. In this case,
that means that when psep is used in a target, it will be ":" instead of
the expected : which can be confusing to read (or worse) the underlying 
shell commands in the build log.

I will make the change on the WIN32 branch of conditional and test when
I get access to a windows system, and read the Windows installing and
building notes.

> In the two cases where this patch "chmod"s source directories so that
> header files can be generated, I think it would be better to generate
> the header files in build directories instead.

I will look into how to do this - I agree, that change would be far more
elegant; I'm not sure if it would be a big change to the ovs build
system as it stands or if it will be simple additional include directory.

> I don't understand why this adds ofp-errors.c and ofp-msgs.c to
> EXTRA_DIST.  These files should be distributed already because they are
> in lib_libopenvswitch_la_SOURCES.

I agree; it's confusing to me as well, since the ofp-errors.c and
ofp-msgs.c aren't so different from (for example) ofp-actions.c or
any of the other libopenvswitch deps. However, it was something I needed
to do to get the build to work - I'll dig around a bit more.

>
> I don't understand the following change, can you explain?
>
>>  .ovsidl.c:
>> -	$(AM_V_GEN)$(OVSDB_IDLC) c-idl-source $< > $@.tmp && mv $@.tmp $@
>> +	$(AM_V_GEN)$(OVSDB_IDLC) c-idl-source $< > $@.tmp && mv -f $@.tmp $@
>>  .ovsidl.h:
>> -	$(AM_V_GEN)$(OVSDB_IDLC) c-idl-header $< > $@.tmp && mv $@.tmp $@
>> +	$(AM_V_GEN)$(OVSDB_IDLC) c-idl-header $< > $@.tmp && mv -f $@.tmp $@
>>  
>>  BUILT_SOURCES += $(OVSIDL_BUILT)
>

If I recall correctly, these files were generated during the dist
and (by distcheck target) marked as read-only. The mv -f will do a 
force move. I will try to dig around further and see what the 
underlying cause for the read-only flag on these files is (or even if
there's an ordering issue or something else in there).

I definitely would like to see distcheck working from the travis-ci side so
that future changes can catch breakage / regression.

-Aaron



More information about the dev mailing list