Skip to content
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

Chakracore build break on VS2013 RC #309

Closed
kunalspathak opened this issue Feb 13, 2016 · 13 comments
Closed

Chakracore build break on VS2013 RC #309

kunalspathak opened this issue Feb 13, 2016 · 13 comments

Comments

@kunalspathak
Copy link
Contributor

I cloned chakracore on a machine that has VS2013 12.0.21005.1 REL. The build fails because VS2013 doesn’t support constexpr that was recently added by @leirocks in 5619992. This is also mentioned in compatibility table and mentioned here.

This is supported starting November 2013 CTP update. Our wiki page

Since wiki page that talks about building chakracore simply list VS2013 as prerequisite but doesn't mention specific version.

I am opening this issue to discuss the guidelines we should have for contributors regarding adding new C++ language features while still maintaining the prerequisite of Visual Studio.

  • If we want contributors to use latest and greatest C++ language features, we should update our wiki page to point the right Visual studio version pre-requisite containing those language features.
  • If want to stick to language features available in RC versions of Visual Studio 2013/2015, then we should point it out in code reviews.

Whatever we decide, we should have some pre-checks build task that verify chakracore builds correctly with VS 2013 and VS 2015 that is mentioned on wiki page. This will help to catch these issues instantly.

@leirocks
Copy link
Contributor

I can remove constexpr if we can’t use it, just move the calculation into the template definition. The only reason I use it is about readability.

Thanks,
Lei

From: Kunal Pathak [mailto:[email protected]]
Sent: Friday, February 12, 2016 4:55 PM
To: Microsoft/ChakraCore [email protected]
Cc: Lei Shi [email protected]
Subject: [ChakraCore] Chakracore build break on VS2013 RC (#309)

I cloned chakracore on a machine that has VS2013 12.0.21005.1 REL. The build fails because VS2013 doesn’t support constexpr that was recently added by @leirockshttps://github.com/leirocks in 56199925619992. This is also mentioned in compatibility tablehttps://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fmsdn.microsoft.com%2flibrary%2fhh567368.aspx%23corelanguagetable&data=01%7c01%7cleish%40microsoft.com%7c6b8503ada07c4412049b08d334104fc7%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=OhjE3A9sk5K%2bUj5366M85LoxRrCPTtODiyHm8wGB9wQ%3d and mentioned herehttps://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fherbsutter.com%2f2013%2f09%2f09%2fvisual-studio-2013-rc-is-now-available%2f%23comment-13540&data=01%7c01%7cleish%40microsoft.com%7c6b8503ada07c4412049b08d334104fc7%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=kVMX5nbrLJdNVXge2rU9rN%2b8aG9J2Nnqp5PvbPZP71c%3d.

This is supported starting November 2013 CTPhttps://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fwww.microsoft.com%2fen-us%2fdownload%2fdetails.aspx%3fid%3d41151&data=01%7c01%7cleish%40microsoft.com%7c6b8503ada07c4412049b08d334104fc7%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=MJU%2bm6G9hS5D5odp5aj%2b%2fP%2b%2bkfgAWBsc6Jsv4qeBAfs%3d update. Our wiki page

Since wiki page that talks about building chakracorehttps://github.com/Microsoft/ChakraCore/wiki/Building-ChakraCore simply list VS2013 as prerequisite but doesn't mention specific version.

I am opening this issue to discuss the guidelines we should have for contributors regarding adding new C++ language features while still maintaining the prerequisite of Visual Studio.

  • If we want contributors to use latest and greatest C++ language features, we should update our wiki page to point the right Visual studio version pre-requisite containing those language features.
  • If want to stick to language features available in RC versions of Visual Studio 2013/2015, then we should point it out in code reviews.

Whatever we decide, we should have some pre-checks build task that verify chakracore builds correctly with VS 2013 and VS 2015 that is mentioned on wiki page. This will help to catch these issues instantly.


Reply to this email directly or view it on GitHubhttps://github.com//issues/309.

@kunalspathak
Copy link
Contributor Author

Sure, we can remove it, but this issue is more about how to handle these things in future.

@curtisman
Copy link
Contributor

We already have daily build to check:
https://github.com/Microsoft/ChakraCore/wiki/Build-Status#legacy-builds
To check all the configuration pre-check in requires a lot of machine resource.
It's ok to detect and fix it quickly for these configurations.

@kunalspathak
Copy link
Contributor Author

@mmitche , it seems that November 2013 update in not installed on Jenkins Win7 build machines that's why legacy builds are failing.

@curtisman - Thanks for clarifying, but we should decide on what should be the guidelines here. @liminzhu, can you please take care of necessary documentation?

@leirocks
Copy link
Contributor

I submitted a PR to remove constexpr usage #310

@dilijev
Copy link
Contributor

dilijev commented Feb 13, 2016

@mmitche Is it possible to update the Win7 machines to the latest Dev12 (with November 2013 update)?

How much use do those machines get outside of our team? Would it be unreasonable for us to ask for configuration changes to more closely model the setup we're anticipating and would like to support on behalf of the community?

@ianwjhalliday
Copy link
Collaborator

Should we bother to continue supporting VS 2013 without the update? It would be nice to have as close to full C++11 support as reasonable.

@liminzhu
Copy link
Collaborator

@kunalspathak, looks like @leirocks is removing constexpr. Is Nov13 update still required?

@kunalspathak
Copy link
Contributor Author

@liminzhu, not now, but might need in future if someone adds new c++ language features.

@mmitche
Copy link
Contributor

mmitche commented Feb 16, 2016

@dilijev For sure. Want to make those changes yourself?

@dilijev
Copy link
Contributor

dilijev commented Aug 25, 2016

@kunalspathak is this still an issue?

@kunalspathak
Copy link
Contributor Author

Haven't tried. Assuming there are no issues, let us close this one and re-open if someone find out build breaks on VS2013.

@dilijev
Copy link
Contributor

dilijev commented Aug 26, 2016

Agreed. Closing as Fixed / No Repro.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants