emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: make dist target for Windows


From: Eli Zaretskii
Subject: Re: make dist target for Windows
Date: Wed, 31 Mar 2010 11:30:51 +0300

> Date: Tue, 30 Mar 2010 21:03:12 -0600
> From: Christoph <address@hidden>
> 
> Find attached a first version of a new make target for the Windows 
> platform: make dist
> 
> This provides a one-click build and packaging for Emacs Windows binary 
> distribution (full and bin only).

Thanks.

> Any feedback appreciated.

Below.

> +echo.   --distfiles             path to files to be included when running 
> make dist, e.g. libXpm.dll

Please make this text shorter, so it fits on a single 80-column line.

> +set distfilename=%~nx1

This is not a good idea.  configure.bat needs to be able to run on
older Windows 9X systems, where %~nx1 is not supported by the stock
shell.  (It is OK to use such advanced constructs in zipdist.bat,
because we can assume that binary distros will not be made on older
systems.)

I actually don't quite understand why you need to use %~nx1: it looks
like distfilename is only used to check for messages to the user, so
you could easily use distfilepath itself.  Is it just for beauty?

>  .PHONY: $(ALL)
>  
> -
>  addpm:                 stamp_BLD $(BLD)/addpm.exe

Please don't make gratuitous white-space changes, at least not as part
of a patch with real changes.

> +     - zipdist.bat $(INSTALL_DIR) $(VERSION)

Why do you ignore errors in this command?  It is the single most
important command in the `dist' target, so perhaps it should really
error out.

> +set ARG_PATH="%~f1"

Suggest SETLOCAL before the first command that sets environment
variables.  Otherwise, you are polluting the user's environment (or
need to unset them all before you exit, which is tedious and
error-prone).

> +set ARG_PATH=%ARG_PATH:\=;%

This warrants a comment, I think.

> +rem Check, if 7zip is installed and available on path
> +:ZIP_CHECK
> +7z a zipcheck zipdist.bat
> +if %ERRORLEVEL% NEQ 0 goto :ZIP_ERROR
> +rm zipcheck.7z

This check is not bulletproof: the command could fail for a reason
that has nothing to do with 7z being available.  For example,
zipdcheck.7z might already exist and be unwritable by the user.

I suggest instead to run 7z without arguments, which should fail only
if 7z cannot be invoked by the user.  As a nice side-effect, you also
get rid of the need to have rm.exe on PATH.

> +rem Build distributions
> +:ZIP_DIST
> +set CUR_DIR=%CD%
> +cd ..\..
> [...]
> +:EXIT
> +cd %CUR_DIR%

Why not use pushd/popd instead?

> +rem Build & verify full distribution

Beware of shell-special characters in REM comments: no one said that
the shell ignores them.  Older Windows shells didn't.  Just use "and"
here, to be safe.

Finally, one more request: I think it would be nice to have a much
smaller *.exe files in the binary distribution, by stripping the debug
info from them.  (We currently use DWARF2 debug info, which is quite
voluminous.)  For all the executables but emacs.exe, this is simple:
just run `strip' on them.  For emacs.exe, you will need to link
temacs.exe with the -s switch to GCC, and then re-dump Emacs.  This is
due to some subtle bug, which no one succeeded to fix yet, that causes
emacs.exe to become an invalid executable file when stripped.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]