-
Notifications
You must be signed in to change notification settings - Fork 757
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Topic/boost 1.61 #2086
Topic/boost 1.61 #2086
Conversation
Would you be so kind to provide a few words what we can expect from this. The rapid boost updates tend to create problems on Windows, especially for VS - which is currently locked out from master following the switch from 1.59 to 1.6. It would be nice to be able to balance win and loss. Locking out VS also means we cannot test if other commits create a problem for VS. Thank you. |
http://www.boost.org/users/history/version_1_61_0.html i use boost dll in supernova. |
It might be good to push this to 3.9 if it is going to block Windows. Or is Windows already blocked ? On Sat, May 14, 2016 at 1:59 AM Tim Blechmann notifications@github.com
|
if people have problems with msvc and boost-1.60, then boost-1.61 won't do any harm, no? people can always use pre-build boost binaries, though. i might also give pointers how to fix build issues, but building boost is as simple as putting all sources into a library. |
The more we advance in a stage where things cannot be tested on a certain platform/with a certain build environment, the bigger the risk is that more commits come in that break a platform. I know there are release notes, what I would like to know is why this is needed for SC if it breaks builds on certain platforms. Again not because I am fundamentally opposed quick updating, but because we need to balance win and loss. Chris, putting it into "a" 3.9 branch only postpones the problem and makes the risk bigger that additional commits only widen the problem for certain platforms. This is community is still too small to maintain two branches well. The longer attention is split between two branches the bigger the risk get's that things get committed that break a platform without anybody noticing or being able to do something about it. It is true that this particular boost update doesn't add to the Windows problem, but the last one did So the real question is general: do we want to accept commits that break a platform or not? Imho the answer is a clear no. The second question is: how to design a release process such that it creates as little as possible disruption for the overall development process. Here one component of the answer is: splitting of a release branch must be done as late as possible. One criteria for creating a release branch could be that all platforms fundamentally work before the branch is split off. Again my reason to vote for such policies is pragmatic: this community is still too small to be able to afford multiple active branches. |
if you don't want to break anything: don't change anything. ever. oh, and if you don't want to risk any bugs: do not try to use visual studio: it is full of known bugs, incorrect and inefficient codegen (real world experience). oh: and if you want to reduce the number of risks: only support one compiler version and one qt version. @bagong even worse than having a small community: supercollider doesn't have any architect. |
In principle, if we're supporting three platforms then one should not push stuff that breaks (some?) builds on one of them without a fix. This is tricky, as not everyone can test (I had to rely on @bagong to help test some stuff on Windows recently, for instance), but in principle it's what we should aim for. I suggest the following:
In future no PRs should be merged that break builds on a platform. That's far more important than any feature/functionality, so there should be a fix in place before merging. That just seems common sense, and it's not fair for Rainer and others to have to fix Windows builds because of changes others wanted. Saying it's already broken by the last time boost was updated, or that VS is crap, are not reasonable arguments for merging this now IMO. If the decision is to not support VS anymore then fine, but again that should be done and in place. |
It was 1.60 that broke the build, 1.61 shouldn't make any difference. Herr Blechmann's arguments are otherwise a bit rough. VS should be able to handle 1.60, no? http://www.boost.org/doc/libs/1_60_0/more/getting_started/windows.html#build-from-the-visual-studio-ide |
Maybe, maybe not. Maybe even probably. But moving targets suck. Better to fix first, then test before merging this. 2 cents (1.2 cents after taxes) |
I am not opposing this merge, it makes no difference for VS, it might actually help a bit (the release notes show that the boost people are aware of problems with VS). But we need that general discussion and a policy. Maybe this is not the moment for debate, but at some point we need it. Lot's of work and time get's anihilated by commits that don't pay attention to "other" platforms. Btw, for the day that this debate is welcome and brought to a policy: I think we need to add some sector of the ARM platform to "supported", e.g. Raspberry Pi or Beaglebone black. They are likely easier to support than VS. But VS is also really important because 90% of prospective developers for Windows will be using VS. Just my 2 cent. |
As to the specific problem for VS somehow related to boost, this is what Luis had to say: "From what I saw on the previous version this was because it [boost] is using a C++11 feature that is not supported by msvc, and it is not clear when it will be. I think that part was trying to use variadic templates." |
Msvc supports most of boost, but there are some things that are not supported. And some of those not supported features are used in sclang. There is a C++11 feature called S_FINAE, that msvc does not support completely, and it is not clear whem they will. I agree with @muellmusik, that the best it's probably to stabilize and get Windows building on master, and then continue from there. |
which part of the release notes are you referring to (disclaimer: i'm
fwiw, "support" will always degrade when nobody actively maintains a this maintenance is an active process, though: if nobody spends time on
i'm using msvc in my professional life. many developers use the standard compliance of the microsoft compiler is very bad, which is the |
@llloret boost is full of broken compiler workarounds for msvc (i'm having a couple of them in my own boost libraries). is SFINAE really used in the sclang codebase? |
Perhaps we can make a quick test? Compile master with the new boost, and see how it compares with the current one, and then we can make a more informed decision about whether to stabilize windows before or after. |
@llloret sounds much better than spreading any FUD |
These are the build errors with boost 1.6.1, VS 2013. I don't remember the details of the build with 1.6.0, but I think they were quite similar:
|
@timblechmann, iirc, the problem were variadic templates, which I think On Sat, 14 May 2016, 12:46 Rainer Schütz, notifications@github.com wrote:
|
@llloret can you try to define variadic templates are "supported" according to https://msdn.microsoft.com/en-US/library/hh567368.aspx. which doesn't mean anything, though. otherwise it should be trivial to replace the asynchronous function calls in |
I can try that tonight, @timblechmann. |
On OSX supernova build breaks here:
|
Something is weird with the travis build. Set to deployment target 10.9 and didn't install libsndfile. I like that supernova is added, but then portaudio needs to be installed too. Oh, you made additions, @timblechmann ... I don't know why deployment target was set to 10.9 rather than 10.7, and why libsndfile wasn't installed. Both shouldn't be case, as far as I know... |
afbe311
to
fd85fdc
Compare
Portaudio! |
Haha, timing overlapp! Sorry... |
Oh, the build system is still Mavericks (10.9). I can't see which XCode version that is. On 10.9 the SuperNova build breaks too, @miczac found that some time ago. I think some time ago we also said it would be good to update the build system to 10.10... This builds for me on El-Capitan/XCode 7.3.1, even with Deployment Target set to 10.7. But: if I run the build on 10.7 the server doesn't start with segmentation fault. SuperNova also doesn't boot, the message is more specific:
Maybe the homebrew el-capitan install of portaudio is not portable to 10.7? |
Or maybe we need to compile portaudio ourselves? ;) |
OSX travis breaks with "pip not found now", seems python needs to be installed via homebrew to be complete and up to date? While travis seems to be showing lot's of problems with this, my conclusion is, that the problems don't have to do anything with the actual PR, which is unproblematic, as far as I can see. It would have been nice to have had a few human readable words why we want it, as it is usual in other PRs, but well...
But it's a warning and it seems pretty superficial. As @llloret was so kind to say he would be willing to look into the predating MSVC problem I think he might look into that at the same time?
The OSX experiments have shown another interesting aspect of the "general" discussion: how conservative to be with toolchains and backwards compatibility. I would suggest to take the travis stuff out of this PR and make a separate one in which we discuss the issues on OSX. I like that SuperNova becomes part of the standard build. But that might imply that we lose 10.7 compatibility, and it means that the releases uses portaudio instead of coreaudio by default. These are important questions that should be discussed appropriately. Maybe using a 10.10 build system will allow to keep 10.7 compatibility, I'll try that... So without the travis stuff this seems good. More needs to be done for travis anyways, so imho it's better to separate that out. |
fyi, there is no 1.6.1, but 1.61.
please note that there is a difference between internal deprecation fwiw, there are much more harmful warnings (e.g. the hidparser warning,
might? please go into details about this. travis builds have been
this is totally wrong please try to verify your claims and make sure to understand what you |
I've described the problem I found with 10.7 support one message before the one you are referring to. I am currently inspecting if they can be avoided by using 10.10/XCode 6.1 as build system.
In my understanding supernova requires portaudio, and the travis builds are used for releases. So would you please explain what is wrong? Tim is this confrontative tone really needed? Don't you see that I am putting a lot of time into supporting your PR? You quote sections of my post, ignoring the other stuff I said about what you quoted. Come on, let's get to a cooperative attitude. I certainly don't mind learning from you, but I really dislike the implicit aggressivity. |
e055392
to
c521799
Compare
;) As opposed to my previous statement I found now that it is possible to build SC with el-capitan/XCode 7.3 such that it runs on 10.7. It is enough to set CMAKE_OSX_DEPLOYMENT_TARGET to 10.7. The "missing SDK" message has no consequences. So changing travis to el-capitan seems to be unproblematic. But - and I stress: unfortunately - adding supernova to the build does break portability. The resulting build does not only not run properly on 10.7, it doesn't even run on 10.10, it only runs on the build machine. My guess is that this has to do with portaudio more than anything else. And there are two issues:
|
my guess is that it is related to using homebrew builds will have a
framework or library? using fixup_bundle might be required:
i won't speculate about crash reports without backtrace. fwiw, i guess someone with an personal interest into osx builds of |
As to the framework/library question, please look earlier in this thread, I posted what I saw. As to your PR, if others agree: the travis changes don't really belong here, but they are okay, pending two changes:
Thank you. |
c521799
to
2d206af
Compare
A separate issue, but I wouldn't make 10.7 support a priority. It's 3 versions and 5 years ago, and was very unpopular. Supporting old OSs is fine but it needs people committed to that.
|
2d206af
to
b75c0f8
Compare
@muellmusik , at this point it costs us nothing to keep 10.7. We've discussed that broadly for Windows. SC shifting it's minimum required version has unexpected side effect, e.g. for sister projects, who might have their own policies. And what about complex installations of which SC is a part, where people would have to spend months to renew their installations. In the end let's not forget the ones that can't afford to update. So to me the most rational consideration that remained in that Windows discussion was: change it if it is objectively necessary to solve a "real" problem. I don't see this here at all... I am sure Scott C. (@scztt) only set it to 10.9 by accident. This setting even excludes 10.8, and I am sure Scott wouldn't do that without consulting others. SuperNova is not locked out for the time being because of the 10.7 setting, it is a portability problem with portaudio, and this even seems to create problems for scsynth. We can build supernova like before, with or without deployment target set to 10.7. |
If it truly costs nothing, then fine of course. If it costs something, that should be weighed. Developer time is a cost. In many cases it's a lot. I'm not saying drop 10.7, but I think it should be low on the priority list as benefits are small and decreasing. In none of the cases mentioned below would people be forced to update to latest SC. In many cases they'd prefer not to.
|
Agreed |
Hi, @timblechmann, regarding your suggestion:
After studying the code, I have tried defining the opposite
If this works we could be one step closer to having a common code base for all OSs, which would be great... Thanks |
This might be a false positive... if I comment the lines that are having the issue, I get other errors. Let me have another look... |
Nah, it does not seem to work with the I want to explore the other approach that @timblechmann suggested:
Tim, can you provide some more guidance? In the world I come from, we don't use std::async not boost::async, so it may be trivial for you, but nor for me :( (But I am eager to learn more). I could look at this myself, but if you have some concrete advise, I could narrow it down quicker. |
Ok, from what I understand, in some places it would be a matter of changing, boost::async with std::async, and then:
with
Does that look about right? |
Ok, I think I have something that seems to work. Although I do not much about that area of the code, and there might be something wrong. I will issue a separate PR so that you can check the code, and see if it looks about right. This would allow to compile sclang under master again. |
@llloret i'd say that |
And what about using std::async with std::launch::async? Seems to be working, and should use a separate thread, right? |
Tim, if I issue a PR, could you have a look? |
@timblechmann, I have issued #2091. Perhaps you can have a look and comment if this is what you had in mind. If not, at least is an initial base that we can work on, to get to a common code base. |
Builds fine on RPi3 and in conjunction with @llloret 's fixes for MSVC also on Windows. I think there is no reason left not to merge. |
b75c0f8
to
4feb981
Compare
So I think this is proven not to create problems. Thanks for maintaining boost, Tim! |
No description provided.