Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

makensis: fix build on 10.9 #24545

Closed
wants to merge 1 commit into from
Closed

Conversation

jodygarnett
Copy link

Compiler and linker flags for OSX 10.9 see issue #23913

@mistydemeo
Copy link
Member

This could probably also be handled by adding ENV.libstdcxx at the top of the install method.

So makensis is just an executable, not a C++ library of any kind?

@mistydemeo
Copy link
Member

Also note that this needs to be scoped to if ENV.compiler == :clang, otherwise it'll break the build when another compiler is being used.

@adamv
Copy link
Contributor

adamv commented Nov 20, 2013

Note: commit messages and pull requests should usually start with name: so we can tell what a change is referring to when scanning down the list, so, makensis: fix build on 10.9 or similar.

@jodygarnett
Copy link
Author

Updated pull request name:

  1. makensis is an executable, it compiles an "nsis" install script into a windows executable. It is nice it runs on mac and linux as well since that is where the developers live.

  2. Not sure how to apply the request to use "ENV.libstdcxx" or "if ENV.compiler == :clang"

@mistydemeo
Copy link
Member

For 2), just add this to the top of the install method

ENV.libstdcxx if ENV.compiler == :clang

And leave the rest alone.

@adamv
Copy link
Contributor

adamv commented Nov 23, 2013

brew-bot is still reporting a build failure here.

@jodygarnett
Copy link
Author

I have no further insight, just followed the request to add ENV.libstdcxx if ENV.compiler == :clang

Do you think this change requires OSX 10.9? What does brew-bot use

@adamv
Copy link
Contributor

adamv commented Nov 24, 2013

Ping @mistydemeo

@mistydemeo
Copy link
Member

The build logs look kinda weird:

scons: warning: The build_dir keyword has been deprecated; use the variant_dir keyword instead.
File "/private/tmp/makensis-5MxS/nsis-2.46-src/SConstruct", line 543, in <module>

scons: warning: The build_dir keyworError: makensis did not build

@matiasgarciaisaia
Copy link

The failed build link seems broken (404). Surely it's old enough to be kept at the server. How can I get it?

This correctly builded in my machine:

$ brew --version
0.9.5
$ scons --version
SCons by Steven Knight et al.:
  script: v2.3.0, 2013/03/03 09:48:35, by garyo on reepicheep
  engine: v2.3.0, 2013/03/03 09:48:35, by garyo on reepicheep
  engine path: ['/usr/local/Cellar/scons/2.3.0/lib/scons/SCons']
Copyright (c) 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013 The SCons Foundation
$ uname -a
Darwin My-MacBook-Pro.local 13.0.0 Darwin Kernel Version 13.0.0: Thu Sep 19 22:22:27 PDT 2013; root:xnu-2422.1.72~6/RELEASE_X86_64 x86_64
$ cd /usr/local/Cellar
$ git remote rename origin official
$ git remote add origin https://github.com/jodygarnett/homebrew
$ git fetch
remote: Counting objects: 34, done.
remote: Compressing objects: 100% (20/20), done.
remote: Total 34 (delta 9), reused 27 (delta 6)
Unpacking objects: 100% (34/34), done.
From https://github.com/jodygarnett/homebrew
 * [new branch]      fix_23913  -> origin/fix_23913
 * [new branch]      gh-pages   -> origin/gh-pages
 * [new branch]      go         -> origin/go
 * [new branch]      master     -> origin/master
$ git checkout fix_23913
Branch fix_23913 set up to track remote branch fix_23913 from origin.
Switched to a new branch 'fix_23913'
$ brew install makensis
==> Downloading http://downloads.sourceforge.net/project/nsis/NSIS%202/2.46/nsis-2.46-src.tar.bz2
Already downloaded: /Library/Caches/Homebrew/makensis-2.46.tar.bz2
==> scons makensis APPEND_CCFLAGS=--stdlib=libstdc++ APPEND_LINKFLAGS=--stdlib=libstdc++
==> Downloading http://downloads.sourceforge.net/project/nsis/NSIS%202/2.46/nsis-2.46.zip
######################################################################## 100.0%
  /usr/local/Cellar/makensis/2.46: 383 files, 7.6M, built in 35 seconds
$ makensis
MakeNSIS v04-Dec-2013.cvs - Copyright 1995-2009 Contributors
See the file COPYING for license details.
Credits can be found in the Users Manual.

Usage:
  makensis [option | script.nsi | - [...]]
   options are:
    -CMDHELP item prints out help for 'item', or lists all commands
    -HDRINFO prints information about what options makensis was compiled with
    -LICENSE prints the makensis software license
    -VERSION prints the makensis version and exits
    -Px sets the compiler process priority, where x is 5=realtime,4=high,
        3=above normal,2=normal,1=below normal,0=idle
    -Vx verbosity where x is 4=all,3=no script,2=no info,1=no warnings,0=none
    -Ofile specifies a text file to log compiler output (default is stdout)
    -PAUSE pauses after execution
    -NOCONFIG disables inclusion of <path to makensis.exe>/nsisconf.nsh
    -NOCD disabled the current directory change to that of the .nsi file
    -Ddefine[=value] defines the symbol "define" for the script [to value]
    -Xscriptcmd executes scriptcmd in script (i.e. "-XOutFile poop.exe")
   parameters are processed by order (-Ddef ins.nsi != ins.nsi -Ddef)
   for script file name, you can use - to read from the standard input
   you can use a double-dash to end options processing: makensis -- -ins.nsi

@matiasgarciaisaia
Copy link

$ git rebase official/master 
First, rewinding head to replay your work on top of it...
Applying: Compiler and linker flags for OSX 10.9
Applying: Update makensis.rb
$ rm /Library/Caches/Homebrew/makensis-*
$ brew reinstall makensis
==> Reinstalling makensis 
==> Downloading http://downloads.sourceforge.net/project/nsis/NSIS%202/2.46/nsis-2.46-src.tar.bz2
######################################################################## 100.0%
==> scons makensis APPEND_CCFLAGS=--stdlib=libstdc++ APPEND_LINKFLAGS=--stdlib=libstdc++
==> Downloading http://downloads.sourceforge.net/project/nsis/NSIS%202/2.46/nsis-2.46.zip
######################################################################## 100.0%
  /usr/local/Cellar/makensis/2.46: 383 files, 7.6M, built in 27 seconds
$ makensis --version
MakeNSIS v04-Dec-2013.cvs - Copyright 1995-2009 Contributors
See the file COPYING for license details.
Credits can be found in the Users Manual.

Usage:
  makensis [option | script.nsi | - [...]]
   options are:
    -CMDHELP item prints out help for 'item', or lists all commands
    -HDRINFO prints information about what options makensis was compiled with
    -LICENSE prints the makensis software license
    -VERSION prints the makensis version and exits
    -Px sets the compiler process priority, where x is 5=realtime,4=high,
        3=above normal,2=normal,1=below normal,0=idle
    -Vx verbosity where x is 4=all,3=no script,2=no info,1=no warnings,0=none
    -Ofile specifies a text file to log compiler output (default is stdout)
    -PAUSE pauses after execution
    -NOCONFIG disables inclusion of <path to makensis.exe>/nsisconf.nsh
    -NOCD disabled the current directory change to that of the .nsi file
    -Ddefine[=value] defines the symbol "define" for the script [to value]
    -Xscriptcmd executes scriptcmd in script (i.e. "-XOutFile poop.exe")
   parameters are processed by order (-Ddef ins.nsi != ins.nsi -Ddef)
   for script file name, you can use - to read from the standard input
   you can use a double-dash to end options processing: makensis -- -ins.nsi

@adamv
Copy link
Contributor

adamv commented Dec 6, 2013

Doesn't the stuff on line 17 also need to be conditional?

@jodygarnett
Copy link
Author

My understanding is that the options on line 17 are fine, and do not need to be conditional. These options explicitly provide what used to be default options. Providing these options (even on earlier versions of OSX) makes it really clear what this build requires.

You re welcome to make them conditional, I do not know enough about home-brew to do this, and I do not have access to an earlier copy of OSX to test.

@adamv
Copy link
Contributor

adamv commented Dec 6, 2013

Can you rebase your changes on master and re-push?

I made a change to force finding Homebrew's versions of scons. The previous build failure is too old and the build bot deleted it.

@adamv
Copy link
Contributor

adamv commented Dec 6, 2013

The problem with the brew output being cut off should also be fixed now, fwiw.

@jodygarnett
Copy link
Author

I am not sure how to rebase, short of checking out everything in git and attacking it like a normal project. All the edits I have provided up until now have been performed using the github "edit" button.

Okay tried that, seems that we are down to a single commit.

See issue Homebrew#23913

Update makensis.rb

Updated based on pull request feedback
@@ -13,7 +13,8 @@ class Makensis < Formula
end

def install
system "scons makensis"
ENV.libstdcxx if ENV.compiler == :clang
system "scons makensis APPEND_CCFLAGS=--stdlib=libstdc++ APPEND_LINKFLAGS=--stdlib=libstdc++"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fail if the compiler isn't clang - GCC doesn't recognize -stdlib. ENV.libstdcxx will take care of this, anyway (it passes -stdlib via CXX, so it makes it through scons).

@mistydemeo
Copy link
Member

Ah, I see the problem... makensis isn't respecting our compiler choice, and I can't find a way to force it to. Unlike mongodb, there's no --cxx= or CXX= option.

@mistydemeo mistydemeo closed this in 6ec705b Dec 7, 2013
@mistydemeo
Copy link
Member

Pushed an alternate fix.

ehershey pushed a commit to ehershey/homebrew that referenced this pull request Apr 4, 2014
makensis only builds with libstdc++. This isn't a huge deal in itself
as it's just a binary, not a library, and has no deps in Homebrew.
More of a problem is the fact that it provides zero mechanism to
govern compiler choice, forcing us to patch the SCons/config.py in
order to make sure the right compiler gets used.

Fixes Homebrew#23913.
Closes Homebrew#24545.
@Homebrew Homebrew locked and limited conversation to collaborators Feb 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants