Menu

#314 portmaker patch

Patch_under_review
closed
portmaker (4)
2012-03-10
2006-11-20
No

Hi,

I attached the portmaker patch with the following fixes:

* lib/mingwPORT.functions: win32path() - use cygpath if pwd -W fails, which is the case on cygwin
* src/portmaker.sh: corrected escaping
* template/mingwPORT.afterquestion: added brackets for proper grouping; the $ARCHIVE.log file was not created in previous version
* template/mingwPORT.configure: call win32path() which woks on msys and cygwin instead pwd -W which is msys specific
* template/mingwPORT.exports: export $CC and $CXX if they are not defined: as "gcc" and "g++" in case of mingw GCC compiler or "gcc -mno-cygwin" and "g++ -mno-cygwin" in case of cygwin GCC compiler

The patched version works on mingw + msys and on cygwin.

Borut

Discussion

  • Earnie Boyd

    Earnie Boyd - 2006-11-20

    Logged In: YES
    user_id=15438
    Originator: NO

    * template/mingwPORT.configure: call win32path() which woks on msys and cygwin instead pwd -W which is msys specific

    Of course this is going to be MSYS specific. MSYS is a required item. I'm going to let Keith make the final call on this patch. I'm thinking that we should reject it though.

     
  • Borut Ražem

    Borut Ražem - 2006-11-20

    Logged In: YES
    user_id=568035
    Originator: YES

    I don't see why it shouldn't work on cygwin too. I can understand that the primary goal is MSYS, but with small changes I made it works with cygwin too and MSYS is not a required item in that case.

    Maybe the problems is that I haven't mentioned clear enough that the result of building on cygwin is "pure mingw" - by using "gcc -mno-cygwin" compiler option, which means the the result is the same as building on msys (actually it is not exactly the same because a different gcc versions is used).

    But it is your decision anyway. If you decide not to include cygwin specifics, then ignore changes in lib/mingwPORT.functions and in template/mingwPORT.exports. Changes in other files are not connected with cygwin and I recommend to apply them.

    Borut

     
  • Borut Ražem

    Borut Ražem - 2006-11-20
     
  • Earnie Boyd

    Earnie Boyd - 2006-11-20

    Logged In: YES
    user_id=15438
    Originator: NO

    Because I don't want to have to deal with the Cygwin related questions on the MinGW lists. Yes, it is possible, mingwPORT is just a shell script but supporting multiple platforms causes an increase in the list traffic. On the other hand I realize that it can increase the visibility of mingwPORT so I might be able to be persuaded differently. Again, I'm going to let Keith Marshall have the final say since he has supplied quite a bit of work into portmaker.

     
  • Keith Marshall

    Keith Marshall - 2006-11-21
    • milestone: --> Patch_under_review
    • assigned_to: nobody --> keithmarshall
     
  • Keith Marshall

    Keith Marshall - 2006-11-21

    Logged In: YES
    user_id=823908
    Originator: NO

    Thanks for your patch, Borut. I will need some time to review this, but some initial comments:--

    1) Like Earnie, I don't want any Cygwin related specifics in the portmaker/mingwPORT core. This is not to say that Cygwin should not be supported as a viable host, but just as I run it on my Linux host, the details of setting that up go in a local configuration file. Thus, in my $HOME/.mingwPORT/mingwPORT.site, which is loaded *after* mingwPORT.functions, I redefine win32path(); this allows me not only to avoid the `pwd -W' issue, but also to emulate the mapping of paths, which MSYS would define in its /etc/fstab, to get Win32 path semantics on a Linux host.

    2) mingwPORT.configure was next on my round tuit list for review. I had intended to make it call win32path() rather than directly use `pwd -W', so we are clearly on the same wavelength with this one.

    3) I hadn't noticed that $ARCHIVE.log was not being created. Thanks for pointing it out.

    4) The `-mno-cygwin' stuff again belongs in mingwPORT.site; I'll need to devise a suitable mechanism to propagate it to the exports list.

    I'll get back to you, after I've had time for a proper review. It has always been my intentention that support for non-MSYS hosts should be facilitated, but in a generic and low profile manner. I will not accept your Cygwin specific patches, but likely will adopt the rest, in some shape or form.

    Thanks again.
    Keith.

     
  • Borut Ražem

    Borut Ražem - 2006-11-21

    Logged In: YES
    user_id=568035
    Originator: YES

    OK, I see your points.

    I agree that using mingwPORT.site for platform specific stuff is a good idea. The only problem is that mingwPORT.site is not available for different platforms...

    But nevertheless, fell free to remove cygwin specific stuff from my patch and I'll create the mingwPORT.site for my personal needs...

    Borut

     
  • Keith Marshall

    Keith Marshall - 2006-11-22

    Sample mingwPORT.site Configuration

     
  • Keith Marshall

    Keith Marshall - 2006-11-22

    Logged In: YES
    user_id=823908
    Originator: NO

    > The only problem is that mingwPORT.site is not available for
    > different platforms...

    I expect any user who is competent enough to *consider* building from mingwPORTs, on any platform other than the intended MSYS, to also be sufficiently competent to set up the necessary environment, by providing *his* *own* mingwPORT.site implementation. I don't intend to *officially* support such use, and therefore, will not be providing preconfigured mingwPORT.site scripts for *any* platform. However, I guess an *example* may be useful; to that end, I've attached *my* current configuration from my GNU/Linux box.

    One comment of note, about this example: this does not redefine win32path(), as I suggested previously. Instead, it redefines `pwd' as an *exported* function. The intent is to make `pwd -W' work within the configure script itself, even on a build host which would not normally support it. This is experimental, and I've yet to test it effectively. The alternative `pwd' is an extended variant of a previously redefined win32path(), and it may be better to revert to that. Even doing so, I would still avoid `cygpath', on a Cygwin host, in favour of the `awk'/`mingwPORT.fstab' technique I've adopted; that way, it is possible to define a virtual mingw32 host configuration, which is independent of the underlying Cygwin host configuration.

    Do note that, mingwPORT.site scripts should *never* be included in any distributed mingwPORT package. This is a definitive policy statement, geared toward our intended objective of not supporting such usage officially. I *will* recommend rejection of any mingwPORT package submission which includes such a script.

     
  • Borut Ražem

    Borut Ražem - 2006-11-22

    Logged In: YES
    user_id=568035
    Originator: YES

    Keith,

    thank you very much for the explanation.

    I took a look to the attached mingwPORT.site.tar.gz and I saw that you moved -mno-cygwin from CC to CFLAGS. Although your solution looks more logical, it doesn't work in all cases with configure: it seems that configure in some cases uses only $(CC) (without $(CFLAGS)) for internal testing, which leads to wrong results if -mno-gcygwin is not included in $(CC). I don't remember all details any more, but I think I had this problem when detecting the presence of sockets on WIN32 in SDCC project...

    (after few minutes ;-) ...probably the problem is when configure runs the preprocessor: it runs $(CC) -E. In this case the wrong header files are included: the ones from cygwin instead those from mingw ...

    The problem about `awk'/`mingwPORT.fstab' is that the mingwPORT.fstab has to be maintained, which is not the case with `cygpath'. One possible solution is to check the presence of `cygpath' and use it or use the `awk'/`mingwPORT.fstab' approach in case `cygpath' doesn't exist. Or the other way around: check the presence of `mingwPORT.fstab' and use it or use `cygpath' if it doesn't exist...

    But anyway, I'm sure you'll find the optimal solution.

    Borut

     
  • Keith Marshall

    Keith Marshall - 2006-11-22

    Logged In: YES
    user_id=823908
    Originator: NO

    > I saw that you moved -mno-cygwin from CC to CFLAGS.

    Actually I didn't. The example I posted is the actual configuration I'm using today, on my Linux box. You'll notice that all the Cygwin related stuff is commented out. It's actually relics of how I configured it myself, about a year ago, to let me very quickly test my own mingwPORT for groff. I rarely use Cygwin now, and I haven't explored that configuration, beyond that quick initial test.

    > Although your solution looks more logical,
    > it doesn't work in all cases with configure: it seems that
    > configure in some cases uses only $(CC) (without $(CFLAGS))

    Perhaps. As I say, I didn't test it in any depth...

    > probably the problem is when configure runs the preprocessor:
    > it runs $(CC) -E. In this case the wrong header files are included:
    > the ones from cygwin instead those from mingw

    ...and this surely would be problematic; your solution of appending `-mno-cygwin' to $CC does seem to be the best choice, and is certainly consistent with advice I've seen elsewhere.

    > The problem about `awk'/`mingwPORT.fstab' is that the mingwPORT.fstab has
    > to be maintained, which is not the case with `cygpath'.

    The problem I see with `cygpath' is that it ties the virtual host configuration to the registry bindings you've defined for your Cygwin host configuration. Even if you have both MinGW/MSYS and Cygwin installed on the same machine, the Cygwin and MSYS path configurations aren't necessarily consistent, although with care, I guess they can be. I prefer to divorce the mingwPORT.site configuration completely from the underlying Cygwin configuration, and to map a generic fstab layout, for MinGW and MSYS installed in the recommended locations. By maintaining a virtual root for that mapping, I can then build a mingwPORT, installing into a completely clean tree, from which I can roll a self contained binary distribution tarball, which I can then unpack into the MinGW tree on any PC, with a standard MinGW/MSYS install in place, and it should just work.

    However, that is purely my preference. I'm not maintaining mingwPORT.site for general distribution; any copy I provide is strictly for example only, and I expect any user adopting it to tweak it to suit their own preferences; if using `cygpath' is more to your taste, then go with it! It's your choice, your configuration; hopefully I've given you the hook, and sufficient guidance, to let you make it work as you prefer.

    > But anyway, I'm sure you'll find the optimal solution.

    What's optimal for me may not suit everyone. No, I'll just leave it to each user to set it up as they wish. Remember, this isn't going to be an officially supported feature; I expect a reasonable level of shell programming competence, from any user adopting it.

     
  • Keith Marshall

    Keith Marshall - 2006-11-23

    Logged In: YES
    user_id=823908
    Originator: NO

    Borut,

    I'm now going to handle your patch. To begin, I've applied this slightly modified variant for mingwPORT.afterquestion:--

    2006-11-23 Borut Razem <borut.razem@siol.net>

    * template/mingwPORT.afterquestion: Add braces;
    they are required to group `tar' and `kill' commands, to raise...
    (SIGUSR1): ...this, on `tar' command failure, while writing...
    ($ARCHIVE.log): ...this file unconditionally.

    Index: template/mingwPORT.afterquestion

    RCS file: /cvsroot/mingw/portmaker/template/mingwPORT.afterquestion,v
    retrieving revision 1.3
    diff -u -r1.3 mingwPORT.afterquestion
    --- template/mingwPORT.afterquestion 9 Sep 2006 09:21:45 -0000 1.3
    +++ template/mingwPORT.afterquestion 23 Nov 2006 19:32:32 -0000
    @@ -95,7 +95,7 @@
    esac
    if cd "$SRCROOT"
    then
    - tar $TARFLAG "$ARCHIVE" || kill -SIGUSR1 $$ | tee "$ARCHIVE.log"
    + { tar $TARFLAG "$ARCHIVE" || kill -SIGUSR1 $$; } | tee "$ARCHIVE.log"
    $ONTRACK || complain afterquestion "tar: $MSG_EFATAL"
    fi
    fi

    BTW, for future reference, please don't include the ChangeLog entry in the diff; it is *much* easier to handle, if you provide it in a separate plain text file.

     
  • Keith Marshall

    Keith Marshall - 2006-11-23

    Logged In: YES
    user_id=823908
    Originator: NO

    Hmm. This:

    \* src/portmaker.sh: corrected escaping
    

    --- portmaker_orig/src/portmaker.sh 2006-11-20 19:11:48.495491200 +0100
    +++ portmaker/src/portmaker.sh 2006-11-20 18:58:58.776741200 +0100
    @@ -19,8 +19,8 @@ ask "DOWNLOADURI" "http://ftp.gnu.org/gn
    ask "SRCDIR" '${SRCROOT-"$HOME/src"}'"/${PACKAGE}-${VERSION}" SRCDIR \?
    ask "BUILDDIR" "./bld" BUILDDIR \?
    ask "PREFIX" '${PREFIX-"/mingw"}' PREFIX \?
    -ask "CFLAGS" "-O3 -s -mms-bitfields -march=\\\`uname -m\\\`" CFLAGS \?
    -ask "CXXFLAGS" "\\\$CFLAGS" CXXFLAGS \?
    +ask "CFLAGS" "-O3 -s -mms-bitfields -march=\`uname -m\`" CFLAGS \?
    +ask "CXXFLAGS" "\$CFLAGS" CXXFLAGS \?

    eval PORTPATH=\"${SRCDIR}/mingwPORT\"
    ask "PORTPATH" "$PORTPATH" PORTPATH \?

    will revert Earnie's:

    2006-02-10 Earnie Boyd <earnie@users.sf.net>

    * src/portmaker.sh (CFLAGS): Add another layer of character quoting
    to prevent the execution of the command during the make process.
    (CXXFLAGS): Ditto.

    I'd like Earnie's comment on this, before I progress this further, as it is not clear to me what command execution problem his patch set out to address.

     
  • Earnie Boyd

    Earnie Boyd - 2006-11-24

    Logged In: YES
    user_id=15438
    Originator: NO

    I'm not at my development computer at the moment. I'm thinking it had to do with needing to carry \ to the Makefile and the number of times the string is evaluated. I can check further on Monday.

     
  • Earnie Boyd

    Earnie Boyd - 2006-11-28

    Logged In: YES
    user_id=15438
    Originator: NO

    My patch was to deal with the issues created by using make to create the portmaker installation. Portmaker.sh is a shell template and is combined with version.sh to create the portmaker end result. See the portmaker target of src/Makefile.

     
  • Keith Marshall

    Keith Marshall - 2006-11-28

    Logged In: YES
    user_id=823908
    Originator: NO

    So maybe, in light of my own subsequent changes to the build procedure, (which removed version.sh, in favour of version.m4), Borut's reversion of that change would be appropriate. The behaviour on my Linux box does seem a little odd now, with that extra level of quoting. I'd like to check it out on a woe32 box to be certain; I'll try to do that tomorrow.

    Regards,
    Keith.

     
  • Keith Marshall

    Keith Marshall - 2006-11-30

    Logged In: YES
    user_id=823908
    Originator: NO

    I've confirmed it now on both MSYS and Linux boxes: the CFLAGS and CXXFLAGS do appear to be overquoted; I believe it will be appropriate to apply Borut's src/portmaker.sh patch, thus reverting the earlier change.

    On a related note, in mingwPORT.ini I see assignments, defined by the portmaker, for both CFLAGS *and* CXXFLAGS, but mingwPORT.question offers the user the opportunity to redefine only CFLAGS. Is there any reason *not* to extend this same courtesy in respect of CXXFLAGS?

     
  • Earnie Boyd

    Earnie Boyd - 2006-11-30

    Logged In: YES
    user_id=15438
    Originator: NO

    No reason I can think of as long as CXXFLAGS default value is the value of CFLAGS as set by the user.

     
  • Keith Marshall

    Keith Marshall - 2006-11-30

    Logged In: YES
    user_id=823908
    Originator: NO

    Would it not be better to make the default equal to whatever the builder of the mingwPORT specified in the mingwPORT.ini?

    By default, portmaker sets that as an *unexpanded* reference to '$CFLAGS', which is then expanded when mingwPORT.question prompts for CXXFLAGS confirmation, so making the default for CXXFLAGS match the user's choice for CFLAGS. In exceptional circumstances, (probably extremely rare), the port builder may wish to add some extra flags to CXXFLAGS, in which case setting the default in mingwPORT.ini as CXXFLAGS='$CFLAGS -modifier-for-cxx ...' should, IMO, be the preferred configuration.

     
  • Earnie Boyd

    Earnie Boyd - 2006-11-30

    Logged In: YES
    user_id=15438
    Originator: NO

    Ok, you convinced me.

     
  • Keith Marshall

    Keith Marshall - 2006-12-03

    Logged In: YES
    user_id=823908
    Originator: NO

    Ok, I committed this:

    2006-12-03 Keith Marshall <keithmarshall@users.sf.net>

    * src/portmaker.sh (CFLAGS, CXXFLAGS): Adjust quoting; fixes an
    overquoting problem reported by Borut Razem.
    * template.mingwPORT.question (CXXFLAGS): Ask user to confirm.

    Index: src/portmaker.sh

    RCS file: /cvsroot/mingw/portmaker/src/portmaker.sh,v
    retrieving revision 1.6
    diff -u -r1.6 portmaker.sh
    --- src/portmaker.sh 9 Sep 2006 09:21:45 -0000 1.6
    +++ src/portmaker.sh 3 Dec 2006 14:46:35 -0000
    @@ -19,8 +19,8 @@
    ask "SRCDIR" '${SRCROOT-"$HOME/src"}'"/${PACKAGE}-${VERSION}" SRCDIR \?
    ask "BUILDDIR" "./bld" BUILDDIR \?
    ask "PREFIX" '${PREFIX-"/mingw"}' PREFIX \?
    -ask "CFLAGS" "-O3 -s -mms-bitfields -march=\\\`uname -m\\\`" CFLAGS \?
    -ask "CXXFLAGS" "\\\$CFLAGS" CXXFLAGS \?
    +ask "CFLAGS" '-O3 -s -mms-bitfields -march=`uname -m`' CFLAGS \?
    +ask "CXXFLAGS" '$CFLAGS' CXXFLAGS \?

    eval PORTPATH=\"${SRCDIR}/mingwPORT\"
    ask "PORTPATH" "$PORTPATH" PORTPATH \?
    Index: template/mingwPORT.question
    ===================================================================
    RCS file: /cvsroot/mingw/portmaker/template/mingwPORT.question,v
    retrieving revision 1.1
    diff -u -r1.1 mingwPORT.question
    --- template/mingwPORT.question 3 Apr 2005 19:45:22 -0000 1.1
    +++ template/mingwPORT.question 3 Dec 2006 14:46:35 -0000
    @@ -41,3 +41,4 @@
    ask "Source path?" "$SRCDIR" SRCDIR
    ask "Installation directory?" $PREFIX PREFIX
    ask "CFLAGS" "$CFLAGS" CFLAGS
    +ask "CXXFLAGS" "$CXXFLAGS" CXXFLAGS

     
  • Keith Marshall

    Keith Marshall - 2012-03-10

    I guess no one is supporting portmaker any further, so there seems to be little point in leaving this open.

     
  • Keith Marshall

    Keith Marshall - 2012-03-10
    • status: open --> closed