Skip to content

Commit

Permalink
Fix a segfault
Browse files Browse the repository at this point in the history
  • Loading branch information
@lukas-w
lukas-w committed Jan 16, 2015
1 parent 4953a9d commit 2257a06
Showing 1 changed file with 6 additions and 6 deletions.
12 changes: 6 additions & 6 deletions src/gui/MainWindow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -530,12 +530,12 @@ void MainWindow::finalize()
}

// Add editor subwindows
for (QWidget* widget : QList<QWidget*>()
<< gui->automationEditor()
<< gui->getBBEditor()
<< gui->pianoRoll()
<< gui->songEditor()
)
for (QWidget* widget : std::list<QWidget*>{
gui->automationEditor(),
gui->getBBEditor(),
gui->pianoRoll(),
gui->songEditor()
})
{
QMdiSubWindow* window = workspace()->addSubWindow(widget);
window->setWindowIcon(widget->windowIcon());
Expand Down

19 comments on commit 2257a06

@tresf
Copy link
Member

@tresf tresf commented on 2257a06 Feb 18, 2015

Choose a reason for hiding this comment

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

@lukas-w this seems to break compilation on OSX Lion (again).

[ 13%] Building CXX object src/CMakeFiles/lmmsobjs.dir/gui/MainWindow.cpp.o
/Users/tresf/lmms/src/gui/MainWindow.cpp:542:26: error: no matching constructor
      for initialization of 'std::list<QWidget *>'
        for (QWidget* widget :  std::list<QWidget*>{
                                ^                  ~
/usr/include/c++/4.2.1/bits/stl_list.h:509:9: note: candidate constructor
      template not viable: requires at most 3 arguments, but 4 were provided
        list(_InputIterator __first, _InputIterator __last,
        ^
/usr/include/c++/4.2.1/bits/stl_list.h:472:7: note: candidate constructor not
      viable: allows at most single argument '__a', but 4 arguments were
      provided
      list(const allocator_type& __a = allocator_type())
      ^
/usr/include/c++/4.2.1/bits/stl_list.h:483:7: note: candidate constructor not
      viable: requires at most 3 arguments, but 4 were provided
      list(size_type __n, const value_type& __value = value_type(),
      ^
/usr/include/c++/4.2.1/bits/stl_list.h:495:7: note: candidate constructor not
      viable: requires single argument '__x', but 4 arguments were provided
      list(const list& __x)
      ^

-Tres

@tresf
Copy link
Member

@tresf tresf commented on 2257a06 Feb 18, 2015

Choose a reason for hiding this comment

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

I'm not sure if this the same C++11 issue as described here or not: #1633

I can certainly switch my build environment to use Homebrew if you think that will fix it.

@lukas-w
Copy link
Member Author

Choose a reason for hiding this comment

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

What GCC version are you using? Your include paths suggest 4.2, which would explain why C++11 isn't working for you.

@tresf
Copy link
Member

@tresf tresf commented on 2257a06 Feb 19, 2015

Choose a reason for hiding this comment

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

I'm not sure. AFAIK, Mac doesn't use GCC at all, so it would be a Clang version returned when I do a g++ --version (etc)

If you think Homebrew will fix this, I'd be happy to switch over to using it, but if it won't help, shouldn't we consider keeping the old syntax for compat reasons? (i.e. Aside from readability, is there any major advantages to the initializer list code?)

@lukas-w
Copy link
Member Author

Choose a reason for hiding this comment

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

The latest Clang version is 3.5.1 so if it says 4.2, that's most likely your GCC version. Initializer lists are supported in GCC 4.4+ and Clang 3.1+.

Maybe this page helps: https://trac.macports.org/wiki/UsingTheRightCompiler The first paragraph says that GCC 4.2 is being used by default with Xcode <4.2, which supports my assumption that you're just using an old compiler. :)

@tresf
Copy link
Member

@tresf tresf commented on 2257a06 Feb 24, 2015

Choose a reason for hiding this comment

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

Switching compilers on OSX is no small feat and LMMS won't build with GCC on Apple anymore so that is not an option.

I can use a newer Clang, but that defeats the purpose of building against Lion as the Clang versions are part of the base SDK.

So what I'm gathering, if we want to build on Lion, we have to hold back on using this feature, correct?

@lukas-w
Copy link
Member Author

Choose a reason for hiding this comment

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

I really don't know anything about how things are on OS X. All I can say is that in #317 we settled on supporting a subset of C++11 features by requiring GCC >=4.6 (see Toby's comment). 4.2.1 is nearly eight years old, we're officially supporting C++11 and are using it all over the place, so maybe you should consider upgrading your compiler :)

LMMS won't build with GCC on Apple anymore

Why's that?

@tresf
Copy link
Member

@tresf tresf commented on 2257a06 Feb 24, 2015

Choose a reason for hiding this comment

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

Something to do with the standard libraries that Clang uses and (IIRC) C++. It has been the cause of a lot of grief (since most of the Clang code fixes could have been circumvented by simply using GCC instead).

Possibly explained here: https://developer.apple.com/library/mac/documentation/DeveloperTools/Conceptual/CppRuntimeEnv/Articles/LibCPPDeployment.html

But I'd be lying if I said that I knew exactly why. I could scroll through my OSX build bug reports to see if I could find the reasoning if you'd like, it will just take a while to locate it. 👍

@tresf
Copy link
Member

@tresf tresf commented on 2257a06 Feb 24, 2015

Choose a reason for hiding this comment

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

so maybe you should consider upgrading your compiler :)

I have a newer build environment, but to maintain backwards compatibility, I chose to setup on Lion. We had several users complaining it wouldn't run and the only downside we have so far to using Lion for making the Mac builds seems to be this bug.

You're right in the idea that it can limit us in terms of using modern compiler features. Please understand that I did spent several days trying to get the old SDK running on a modern environment to no avail. I think I have this documented as well.

What I'm not sure about is whether or not the SDK implies the older Clang version or not, so this may be moot.

I've only begun to scrape the surface of C++ so I'm sorry I don't know more about these things, but I can say with all certainty that the project has had positive reception of our "eight year old" SDK builds made with OSX Lion. Furthermore, building on Lion closes #555 (comment) which is another good reason to keep it around IMO. 😕

-Tres

@badosu
Copy link
Contributor

@badosu badosu commented on 2257a06 Feb 24, 2015

Choose a reason for hiding this comment

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

I wish I'd know anything about OSX to be able to help here. But I don't understand how there's not a syntax that works on the different versions.

@tresf
Copy link
Member

@tresf tresf commented on 2257a06 Feb 24, 2015

Choose a reason for hiding this comment

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

But I don't understand how there's not a syntax that works on the different versions.

There is here: #1633 (comment)

Lukas is just advocating for enabling (enforcing?) the standards that we have already adopted. 😏

@badosu
Copy link
Contributor

@badosu badosu commented on 2257a06 Feb 24, 2015

Choose a reason for hiding this comment

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

Oh, ok. I'd prefer to have a modern build line for OSX, but if that's not possible I guess that's the way to go.

@tresf
Copy link
Member

@tresf tresf commented on 2257a06 Feb 24, 2015

Choose a reason for hiding this comment

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

Oh, ok. I'd prefer to have a modern build line for OSX, but if that's not possible I guess that's the way to go.

It's a matter of appeasing our users I think per #1270, #1227. It's not much different than the XP debate... although....

  • Lion Release Date: July 1, 2011
  • XP Release Date: August 24, 2001

😃

-Tres

@tresf
Copy link
Member

@tresf tresf commented on 2257a06 Feb 24, 2015

Choose a reason for hiding this comment

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

To my own point though, I could consider upgrading my build environment to Mountain Lion + standard SDK version, which claims to have backwards compat with Lion...

@badosu
Copy link
Contributor

@badosu badosu commented on 2257a06 Feb 24, 2015

Choose a reason for hiding this comment

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

@tresf I am not familiar with our build toolchain, I am mostly concerned with Linux builds, so I really don't know what you're talking about :-). Only the changes that happen on the source code as derived from these discussions I try to understand.

@tresf
Copy link
Member

@tresf tresf commented on 2257a06 Feb 24, 2015

Choose a reason for hiding this comment

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

I really don't know what you're talking about :-)

I think that is the point of the discussion, to share this type of knowledge.

We've had enough requests via Facebook, Google+ and the bug tracker to justify building for OSX 10.7 "Lion", but when we use the modern build line for OSX (i.e. 10.10 "Mavericks") it simply won't run on Lion. It crashes.

What I'm saying is I can take the time to switch to OSX 10.7 "Mountain Lion" with the hopes it will remedy this issue, or we can patch this code for backwards compat... or I can patch it every time I build.

Naturally, my vote is to patch it for backwards compat because it is of minimal effort and saves me a lot of work. But I'm trying not to be selfish here. 😄

-Tres

@badosu
Copy link
Contributor

@badosu badosu commented on 2257a06 Feb 24, 2015

Choose a reason for hiding this comment

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

Yeah, I agree with the retrocompatibility patch. Maybe you could send a PR for this and continue this discussion there?

Also, we should make the build system be suited to match our current release system, so we can capture these issues as they happen.

If I am not mistaken, some build systems provide an artifact retrieval system, this means that after a build has occurred we can download the binary created by it. It woud be nice if we could use this instead of depending on a collaborator's setup.

@lukas-w
Copy link
Member Author

Choose a reason for hiding this comment

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

It really shouldn't be a problem to target an "older" OS using a newer compiler.

it simply won't run on Lion. It crashes.

You probably didn't ship the C++ standard library (likely named libstdc++.so) with the build. Doing so ensures that LMMS is using the library version that was used during building, not the one present on the target system (i.e. v4.2 on Lion as it appears).

@tresf
Copy link
Member

@tresf tresf commented on 2257a06 Feb 25, 2015

Choose a reason for hiding this comment

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

It really shouldn't be a problem to target an "older" OS using a newer compiler.

Unless someone can help me get further than this (#1270 (comment)), it won't happen. :)

I contacted the MacPorts team and to summarize our conversation, what we're trying to achieve with cross-compilation is not officially supported and also not recommended.

This is the point in which I switched to a Lion build machine. I'm open for ideas, I just feel we're at an ideal spot with Lion builds.

You probably didn't ship the C++ standard library (likely named libstdc++.so) with the build. Doing so ensures that LMMS is using the library version that was used during building, not the one present on the target system (i.e. v4.2 on Lion as it appears).

Yeah, I'm not sure how to do that. Here's the shell script I use to create the bundle. data/scripts/create_apple_bundle.sh.in If you think it is worth adding and linking please steer me in the right direction and I can take a swing at it.

On a side note... the DMG script doesn't work on Lion so I'd like to move off of the platform, but I also want to be practical. 🍺

Please sign in to comment.