Re: [msysGit] [PATCH] Embed an application manifest on MinGW

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: May 28, 2011, 21:19 UTC
Message-ID: <alpine.DEB.1.00.1105282311220.11411@bonsai2>
In-Reply-To: <4DE16418.4020002@cesarb.net>

Hi,

On Sat, 28 May 2011, Cesar Eduardo Barros wrote:

Show 30 quoted lines

> Em 28-05-2011 17:44, Johannes Schindelin escreveu:
> > On Sat, 28 May 2011, Cesar Eduardo Barros wrote:
> > > [...]
> > > @@ -1551,6 +1554,11 @@ ifdef USE_NED_ALLOCATOR
> > >          COMPAT_OBJS += compat/nedmalloc/nedmalloc.o
> > >   endif
> > >
> > > +ifdef APPLICATION_MANIFEST
> > > +	# Cannot be in LIB_OBJS because it must always be linked in
> > > +	RESOURCE_OBJS += compat/win32/resource.o
> > > +endif
> > > +
> > >   ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT
> > >   	export GIT_TEST_CMP_USE_COPIED_CONTEXT
> > >   endif
> > > [...]
> > >
> > > -git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
> > > +git$X: git.o $(BUILTIN_OBJS) $(RESOURCE_OBJS) $(GITLIBS)
> > > [...]
> > >
> > > -git-%$X: %.o $(GITLIBS)
> > > +git-%$X: %.o $(RESOURCE_OBJS) $(GITLIBS)
> > > [...]
> >
> > Is it possible that adding the resource object to GITLIBS would reduce 
> > the patch and be "more correct", too?
> 
> I thought about it, but it felt wrong. It is not a library, it is an 
> object.

Well, a library is a collection of objects. A .a file traditionally contains also .o files that define both global and local variables.

Even so, let's assume for a moment that you're right and a library is a library of functions (which I assume that you assume), and let's ignore for a moment that GITLIBS provides non-functions such as inside_git_dir and auto_crlf. Then the approach of adding another $(RESOURCE) in exactly all the places where we have $(GITLIBS) is so redundant that it asks for future inconsistencies (somebody _will_ forget, because things still compile). In that case, one patch to rename GITLIBS to, say, COMMON_OBJECTS, and another patch to add the manifest to the common objects.

But as I said, IMHO GITLIBS is aptly named and should take the resource, too.

Show 6 quoted lines

> Also, while adding it to GITLIBS would add the object to all the right 
> rules (in fact, I did a search for GITLIBS to find all the places to add 
> the new object to), I believe it would get the object linked twice in 
> some of the rules (the ones which do a $(filter %.o,$^) would get it 
> both from GITLIBS via the filter and from GITLIBS via $(LIBS) on the 
> command line).

Does it result in a link error? If not, it's not an issue.

Ciao, Johannes