You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by Liviu Nicoara <ni...@roguewave.com> on 2005/12/15 22:36:38 UTC

makefile patch

Martin,

Could you please take a look at this patch and let me know if it meets
your approval?

I have removed the all target, replaced it with a lib, which being first
in the makefile is picked when building w/ no goals and configures and
builds the library (no examples and/or tests which I believe it was the
intention).

I removed the useless builddir target in TOPDIR.

I added comments.

I added dependencies in the BUILDDIR invocations of make (else branch).

Thanks,
Liviu


Re: [PATCH] build process fails in TOPDIR if target (lib, etc.) specified (was Re: makefile patch)

Posted by Liviu Nicoara <ni...@roguewave.com>.
>>Index: etc/config/makefile.rules
>>===================================================================
>>--- etc/config/makefile.rules	(revision 357098)
>>+++ etc/config/makefile.rules	(working copy)
>>@@ -157,7 +157,10 @@
>> # move IBM xlC generated .u files to the respective .d files
>> 
>> define makedep
>>-    -@(if [ ! -f $@ ] ; then                                                 \
>>+    -@(if [ ! -f $(INCDIR)/config.h ] ; then                                 \
>>+           $(MAKE) -C$(INCDIR);                                              \
>>+       fi;                                                                   \
>>+       if [ ! -f $@ ] ; then                                                 \
> 
> 
> Would it be possible to make this a dependency of the $(DEPENDDIR)/%.d
> target below instead? I.e., have the .d files depend on config.h...

Yes, it is possible (actually this is the first I tried) but it means
changes to three deps rules  and the addition of a special rule for that
config.h file. If you add more branches to the conditionals governing
the creation of the .d files then you have to add the same dependency to
them too.

This one is the simplest change going to the point where all the deps
rules converge.

Liviu

Re: [PATCH] build process fails in TOPDIR if target (lib, etc.) specified (was Re: makefile patch)

Posted by Martin Sebor <se...@roguewave.com>.
Liviu Nicoara wrote:
> Addressing:
> 
> http://issues.apache.org/jira/browse/STDCXX-85
> 
> ChangeLog entry:
> 
> 2005-12-15  Liviu Nicoara  <ni...@roguewave.com>
> 
         STDCXX-85
         ^^^^^^^^^
This needs to be up here for the Jira Subversion plugin to be able
to associate the change with the issue.

> 	* GNUMakefile: removed useless rules: all, builddir, added
> 	  rule lib, added variable INCDIR
> 	* etc/config/GNUmakefile.lib: invoked make config when
> 	  $(INCDIR)/config.h is missing at top of makedep macro
[...]
> Index: etc/config/makefile.rules
> ===================================================================
> --- etc/config/makefile.rules	(revision 357098)
> +++ etc/config/makefile.rules	(working copy)
> @@ -157,7 +157,10 @@
>  # move IBM xlC generated .u files to the respective .d files
>  
>  define makedep
> -    -@(if [ ! -f $@ ] ; then                                                 \
> +    -@(if [ ! -f $(INCDIR)/config.h ] ; then                                 \
> +           $(MAKE) -C$(INCDIR);                                              \
> +       fi;                                                                   \
> +       if [ ! -f $@ ] ; then                                                 \

Would it be possible to make this a dependency of the $(DEPENDDIR)/%.d
target below instead? I.e., have the .d files depend on config.h...

>             if [ ! -d $(DEPENDDIR) ]; then                                    \
>                 mkdir -p $(DEPENDDIR);                                        \
>             fi;                                                               \
> @@ -196,14 +199,12 @@
>  $(DEPENDDIR)/%.d: %$(AS_EXT)
>  	$(makedep)

...like so:

+  $(DEPENDDIR)/%.d: %$(AS_EXT) $(INCDIR)/config.h
+  	$(makedep)

Martin

[PATCH] build process fails in TOPDIR if target (lib, etc.) specified (was Re: makefile patch)

Posted by Liviu Nicoara <ni...@roguewave.com>.
Addressing:

http://issues.apache.org/jira/browse/STDCXX-85

ChangeLog entry:

2005-12-15  Liviu Nicoara  <ni...@roguewave.com>

	* GNUMakefile: removed useless rules: all, builddir, added
	  rule lib, added variable INCDIR
	* etc/config/GNUmakefile.lib: invoked make config when
	  $(INCDIR)/config.h is missing at top of makedep macro


Re: makefile patch

Posted by Martin Sebor <se...@roguewave.com>.
Liviu Nicoara wrote:
> Martin Sebor wrote:
> 
>>Liviu Nicoara wrote:
>>
>>
>>>>>I think we had those at some point in the past. IIRC, I took them out
>>>>>because I thought it was cleaner to have them in the individual (sub)
>>>>>makefiles instead. I.e., the top level makefile shouldn't need to know
>>>>>all the dependencies of each subcomponent. It easy to miss some (such
>>>>>as the dependency of tests on rwtest).
> 
> 
>>I see. Would including the .d files conditionally, only if config.h
>>exists, be a way to solve the problem?
> 
> 
> Not really. It would mean we'd build the library w/o the dependencies.

Yeah, and it would be a hack, too.

It seems the target that generates the .d files should depend on
config.h and cause it to be generated if it doesn't exist. That
way we'd build with the dependencies even the first time through.

Martin

Re: makefile patch

Posted by Liviu Nicoara <ni...@roguewave.com>.
Martin Sebor wrote:
> Liviu Nicoara wrote:
> 
>>>>I think we had those at some point in the past. IIRC, I took them out
>>>>because I thought it was cleaner to have them in the individual (sub)
>>>>makefiles instead. I.e., the top level makefile shouldn't need to know
>>>>all the dependencies of each subcomponent. It easy to miss some (such
>>>>as the dependency of tests on rwtest).

> I see. Would including the .d files conditionally, only if config.h
> exists, be a way to solve the problem?

Not really. It would mean we'd build the library w/o the dependencies.

Liviu

Re: makefile patch

Posted by Martin Sebor <se...@roguewave.com>.
Liviu Nicoara wrote:
>>>I think we had those at some point in the past. IIRC, I took them out
>>>because I thought it was cleaner to have them in the individual (sub)
>>>makefiles instead. I.e., the top level makefile shouldn't need to know
>>>all the dependencies of each subcomponent. It easy to miss some (such
>>>as the dependency of tests on rwtest).
> 
> 
> Funny thing... I believe the makefiles are too smart. :-)
> 
> I believe there is a problem with completely removing the all/lib/etc.
> dependencies listed in top level GNUmakefile:
> 
> 1. invoke make in topdir with target lib (or no target & pick lib as
>    first target)
> 2. lib target invokes make recursively in BUILDDIR w/ target lib
> 3. target lib has no deps, so invokes make on dir lib where the makefile
>    is GNUmakefile.lib
> 4. GNUmakefile.lib includes *.d files; the rule for building them gets
>    invoked before any other target is considered
> 
> Step 4 fails because config.h is missing.

I see. Would including the .d files conditionally, only if config.h
exists, be a way to solve the problem?

Martin

> 
> If I have the deps listed in the top level GNU makefile then the config
> is executed first, followed by the actual target passed to it.
> 
> It is possible that any other target we try to build starting from
> TOPDIR will fail in a similar way - haven't tried.
> 
> So, the long story short, I don't think I can change this flow without
> some major makefile reorganization. I believe we should go ahead with
> the explicit deps in top level GNUmakefile.
> 
> Liviu


Re: makefile patch

Posted by Liviu Nicoara <ni...@roguewave.com>.
>>I think we had those at some point in the past. IIRC, I took them out
>>because I thought it was cleaner to have them in the individual (sub)
>>makefiles instead. I.e., the top level makefile shouldn't need to know
>>all the dependencies of each subcomponent. It easy to miss some (such
>>as the dependency of tests on rwtest).

Funny thing... I believe the makefiles are too smart. :-)

I believe there is a problem with completely removing the all/lib/etc.
dependencies listed in top level GNUmakefile:

1. invoke make in topdir with target lib (or no target & pick lib as
   first target)
2. lib target invokes make recursively in BUILDDIR w/ target lib
3. target lib has no deps, so invokes make on dir lib where the makefile
   is GNUmakefile.lib
4. GNUmakefile.lib includes *.d files; the rule for building them gets
   invoked before any other target is considered

Step 4 fails because config.h is missing.

If I have the deps listed in the top level GNU makefile then the config
is executed first, followed by the actual target passed to it.

It is possible that any other target we try to build starting from
TOPDIR will fail in a similar way - haven't tried.

So, the long story short, I don't think I can change this flow without
some major makefile reorganization. I believe we should go ahead with
the explicit deps in top level GNUmakefile.

Liviu

Re: makefile patch

Posted by Liviu Nicoara <ni...@roguewave.com>.
Martin Sebor wrote:
> Liviu Nicoara wrote:
> 
> But first a few things about the process :)

Will follow.

> 
> That looks okay. I'm just not too excited about the duplication
> if the code between lib and .DEFAULT. Is there an easy way to
> avoid it?

Not excited about that either. I spent some time thinking about it but I
cannot find an easier way in that many lines of code.

> 
> 
>>I removed the useless builddir target in TOPDIR.
> 
> 
> I guess that's okay, too, except that we will now also need to remove
> the comment explaining builddir. I'm not sure why it was even there if
> it didn't do anything. How does BUILDDIR get created now? By including
> makefile.in?

Yep, it always did it this way.

>>I added dependencies in the BUILDDIR invocations of make (else branch).
> 
> 
> I think we had those at some point in the past. IIRC, I took them out
> because I thought it was cleaner to have them in the individual (sub)
> makefiles instead. I.e., the top level makefile shouldn't need to know
> all the dependencies of each subcomponent. It easy to miss some (such
> as the dependency of tests on rwtest).

I will look into it.

Liviu

Re: makefile patch

Posted by Martin Sebor <se...@roguewave.com>.
Liviu Nicoara wrote:
> Martin,
> 
> Could you please take a look at this patch and let me know if it meets
> your approval?

Will do.

But first a few things about the process :)

1. The subject line of a patch should start with the string [PATCH]
to make it easily distinguishable from other posts.

2. If you expect me (or others) to try the patch out and give you
comments it must be possible to apply it using the patch utility.
In this case you it helps to also describe what the patch does
and why.

3. If you expect someone else to apply the patch you also need to
submit a ChangeLog entry in the usual format describing the change.
If the patch fixes a bug you need make sure to include a reference
to the bug key. This patch fixes STDCXX-85, correct?
http://issues.apache.org/jira/browse/STDCXX-85

See also: http://incubator.apache.org/stdcxx/bugs.html#patches

> 
> I have removed the all target, replaced it with a lib, which being first
> in the makefile is picked when building w/ no goals and configures and
> builds the library (no examples and/or tests which I believe it was the
> intention).

That looks okay. I'm just not too excited about the duplication
if the code between lib and .DEFAULT. Is there an easy way to
avoid it?

> 
> I removed the useless builddir target in TOPDIR.

I guess that's okay, too, except that we will now also need to remove
the comment explaining builddir. I'm not sure why it was even there if
it didn't do anything. How does BUILDDIR get created now? By including
makefile.in?

> 
> I added comments.

Those ate always good! :)

> 
> I added dependencies in the BUILDDIR invocations of make (else branch).

I think we had those at some point in the past. IIRC, I took them out
because I thought it was cleaner to have them in the individual (sub)
makefiles instead. I.e., the top level makefile shouldn't need to know
all the dependencies of each subcomponent. It easy to miss some (such
as the dependency of tests on rwtest).

Martin