[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