Re: [patch/rebase] Fix build problems in imagehelper

[Charles Wilson <cygwin at cwilson dot fastmail dot fm>]

On 6/20/2011 12:22 PM, Corinna Vinschen wrote:
> On Jun 20 11:52, Jason Tishler wrote:
>> I would like to give Chuck a chance to apply the following patch that he
>> had already submitted (so it applies cleanly):
>>
>> * autoconfify rebase and imagehelper
>> * make peflagsall and rebaseall set PATH to
>>     (dir-of-$0):(configured $bindir):/bin
>>   for better relocate-ability.
>> * fix a number of warnings related to deprecated cygwin functions;
>>   use cygwin_create_path rather than cygwin_conv_, if available. (1)
>> * add some fixes/workarounds for MSYS
>> * some accommodations to use explicitly 'dash' rather than 'ash',
>>   if desired.
>> * Add explicit COPYING file for GPLv2.

Perhaps we should consider using GPLv3+ at this point?  Can we contact
Ralf Habacker for permission, with regards to libimagehelp?

>> * Update build.sh to accommodate MinGW and MSYS, as well as Cygwin.
>> * Add getopt_long implementation to workaround broken impl on MSYS
>>   (only built if $host=MSYS)
>>
>> Is that OK?
> 
> No worries, I can wait.
> 
> Chuck, I would rather use cygwin_conv_path together with a static buffer
> instead of cygwin_create_path.  Win32Path is called within the
> CreateFile argument list.  The memory allocated in Win32Path is never
> free'd...

You mean a single 32K (see below) buffer, on the stack? Yikes...  We'd
be better off with a global static variable, malloc'ed in main().

> Also, it would be nice to call CreateFileW rather than CreateFile to
> accommodate potential native characters in the pathname.  I already have
> a local patch which calls setlocale in rebase to support this.

...and this is why you'd need 32K instead of "merely" 4k, right?

I have no real issue with these changes -- other than concerns about how
big our stack space is allowed to be in a single frame -- but I'd prefer
to handle them as a follow-on. This patch is already big and messy
enough as it is.

--
Chuck

quote from Jun 2010 email:

There was quite a bit of renaming files, so I've listed the 'cvs rm' and
'cvs add' files below.

=========== Toplevel ChangeLog ================
2010-06-02  Charles Wilson  <...>

	Use autoconf. Configurable installation and SHELL.

	* ChangeLog: New.
	* COPYING: New.
	* Changes: Renamed to...
	* NEWS: this.
	* build-aux/config.guess: New.
	* build-aux/config.sub: New.
	* build-aux/install-sh: New.
	* Makefile: Removed.
	* build.sh: Run configure. Modify to support MinGW
	and MSYS, as well as Cygwin.
	* configure.ac: New.
	* Makefile.in: Rewrite for autoconf.
	* peflagsall: Renamed to...
	* peflagsall.in: this. Set PATH to directory of $0,
	$(bindir), and /bin. Exclude dash as well as ash
	from rebaselist, and peflags.exe itself. Don't die
	if dash processes are running.
	* rebaseall: Renamed to...
	* rebaseall.in: this. Set PATH to directory of $0,
	$(bindir), and /bin. Exclude dash as well as ash
	from rebaselist, and rebase.exe itself.
	* peflags.c: Accomodate MSYS.
	* getopt.h_: Work around broken getopt_long on MSYS.
	* getopt_long.c: Ditto.

Prior: see NEWS file.

=========== imagehelper ChangeLog ================
2010-06-02  Charles Wilson  <...>

        Use autoconf. Treat as intrinsic part of rebase package,
        rather than almost-but-not-really independent. Correct
        gcc warnings.

        * ChangeLog: Fix whitespace and formatting.
        * INSTALL: Removed.
        * Makefile: Removed.
        * Makefile.cygwin: Removed.
        * Makefile.mingw: Removed.
        * Makefile.in: Rewrite for autoconf. Bump library version
        due to changes below.
        * rebase.h: Removed unused file.
        * sections.cc: Silence gcc warnings.
        * sections.h: Silence gcc warnings.
        * objectfile.cc: Use cygwin_create_path() when available.
        Accomodate MSYS.
        * rebase_main.cc: Ditto.
        * rebind_main.cc: Ditto.
        * unbind_main.cc: Ditto.

===========================================================
New files (cvs add):
	ChangeLog COPYING NEWS build-aux/config.guess
	build-aux/config.sub build-aux/install-sh
	configure.ac peflagsall.in rebaseall.in
	getopt.h_ getopt_long.c

Removed files (cvs rm):
	Changes Makefile peflagsall rebaseall
	imagehelper/INSTALL imagehelper/Makefile
	imagehelper/Makefile.cygwin imagehelper/Makefile.mingw
	imagehelper/rebase.h

Modified files:
	build.sh Makefile.in peflags.c
	imagehelper/ChangeLog imagehelper/Makefile.in
	imagehelper/sections.cc imagehelper/sections.h
	imagehelper/objectfile.cc imagehelper/rebase_main.cc
	imagehelper/rebind_main.cc imagehelper/unbind_main.cc

I've attached:
  1) the patch, which you can apply to your CVS repo and then do
     the 'cvs rm', 'cvs add', 'cvs commit' as described above, and
  2) the resulting src tarball, which you could also create by doing:
        autoreconf -fvi
        ./configure
        make dist
     but I attach it so that you can do a before-after diff to ensure
     that everything is applied correctly.
============== end quote ===============

I have NOT attached the full "3.0.2" tarball, basically to avoid
spamming the list with a very large attachment.

Attachment: rebase-autoconf.patch.bz2
Description: Binary data