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: Christoph
Subject: Re: make dist target for Windows
Date: Wed, 31 Mar 2010 19:48:44 -0600
User-agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.9) Gecko/20100317 Thunderbird/3.0.4

On 3/31/2010 2:30 AM, Eli Zaretskii wrote:
+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.
Done.

+set distfilename=%~nx1
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?
Yes, just to show the filename. I removed all of that and just display a generic message.

  .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.
Sorry, about that. That was not intentional.

+       - 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.
Yes it should. Fixed.

+set ARG_PATH="%~f1"
Suggest SETLOCAL before the first command that sets environment
variables.
Good point. Added.

+set ARG_PATH=%ARG_PATH:\=;%
This warrants a comment, I think.
I had some trouble parsing the path with '\' in it. Someone suggested replacing it and it worked miraculously. Still would like to know why the '\' didnt work, but my batch-fu is not that great. Do you have any idea?

+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
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.
Agreed. This is a better solution.

+rem Build distributions
+:ZIP_DIST
+set CUR_DIR=%CD%
+cd ..\..
[...]
+:EXIT
+cd %CUR_DIR%
Why not use pushd/popd instead?
Ignorance, I guess. ;) Fixed.

+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.
Good point. Didn't think of that. Fixed.


v2 of the patch with all the fixes above is attached.

Thanks,
Christoph

Attachment: makedistw32v2.txt
Description: Text document


reply via email to

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