search

cmu-db / peloton

The Self-Driving Database Management System
http://pelotondb.io
Apache License 2.0
2.03k stars 624 forks source link

Fix warning for unused lambda capture on Clang 9.1 #1400

Closed mbutrovich closed 6 years ago

mbutrovich commented 6 years ago

Resolves the following warning introduced by #1371 on Clang 9.1:

../git/peloton/test/codegen/csv_scan_translator_test.cpp:39:30: error: lambda capture 'quote' is not required to be captured for this use [-Werror,-Wunused-lambda-capture] const auto quote_string = [quote](std::string s) {

I couldn't reproduce the warning on GCC 7.3, which makes me wonder if it's spurious on Clang's part. However, I ran make check with this proposed change under Clang 9.1 and GCC 7.3 and everything looked good. Hopefully it's the same story on Jenkins and Travis.

pmenon commented 6 years ago

LGTM.

@pervazea What is the status of getting the newer Mac onto the Jenkins infrastructure?

pervazea commented 6 years ago

We provided the Mac to Chad, so it could be installed in the PDL lab. I haven't heard an update on this... will check on it later today, to see how it is progressing.

From: Prashanth Menon notifications@github.com Sent: Tuesday, June 12, 2018 10:48 AM To: cmu-db/peloton Cc: Pervaze Akhtar; Mention Subject: Re: [cmu-db/peloton] Fix warning for unused lambda capture on Clang 9.1 (#1400)

LGTM.

@pervazeahttps://github.com/pervazea What is the status of getting the newer Mac onto the Jenkins infrastructure?

- You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/cmu-db/peloton/pull/1400#issuecomment-396617120, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AhDfwGQZFv1zze9LM6UqwTVc40TLAbJrks5t79TXgaJpZM4UjEy2.

crd477 commented 6 years ago

How important is it to isolate the build process on this platform? The problem is that, unlike the current Docker setup for Linux builds, PRs from random users would get merged and built directly on the system.

The approach I'd like to take is to run macOS inside VirtualBox on this system and install the Jenkins agent on the VM there but this will take me a bit of time. Is that OK?

apavlo commented 6 years ago

@crd477 It's probably best to play it safe and run the builds in a sandbox.

But doesn't mean if we can run OSX in a VM then we don't need Mac hardware in the future?

crd477 commented 6 years ago

No. Section 2.B.iii of The software license agreement for macOS states:

(iii) to install, use and run up to two (2) additional copies or instances of the Apple Software within virtual operating system environments on each Mac Computer you own or control that is already running the Apple Software, for purposes of: (a) software development; (b) testing during software development; (c) using macOS Server; or (d) personal, non-commercial use.

So running several macOS VMs in a hypervisor under macOS running natively on the Apple hardware appears to be permitted. That's the way I propose to set it up.

[EDIT: it only occurs to me clearly now that I reread what I just posted that I should get official guidance from computing services, which I will do right now...]

crd477 commented 6 years ago

OK, computing services says:

Software Licensing just updated the ticket and said they're okay with the virtual machines. They believe that your description of your intent is within the terms of the license agreement. Thanks!

I'm intend to make some progress on this today.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.004%) to 76.975% when pulling 482338b563f4b6fca0113f3e77481386fed3fa5f on mbutrovich:clang_fix into 12440023174a2605e9a024f95390c168f7557b83 on cmu-db:master.

crd477 commented 6 years ago

The macOS VM has been added to the pool. Does the build stage that I specified for macOS look OK to you? If so, I'll generate a PR. This systems seems to perform OK with a single VM and 4 Jenkins executors. If you want to get more macOS capacity, I think it will be necessary to buy some more minis and reuse the scripts I have in place currently.

pervazea commented 6 years ago

Yes, macOS build stage looks ok.

Pervaze

From: Chad Dougherty notifications@github.com Sent: Friday, July 6, 2018 9:15:13 AM To: cmu-db/peloton Cc: Pervaze Akhtar; Mention Subject: Re: [cmu-db/peloton] Fix warning for unused lambda capture on Clang 9.1 (#1400)

The macOS VM has been added to the pool. Does the build stage that I specified for macOShttp://jenkins.db.cs.cmu.edu:8080/blue/organizations/jenkins/crd%2Fpeloton-crd/detail/mac-jenkins/10/pipeline/16 look OK to you? If so, I'll generate a PR. This systems seems to perform OK with a single VM and 4 Jenkins executors. If you want to get more macOS capacity, I think it will be necessary to buy some more minis and reuse the scripts I have in place currently.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/cmu-db/peloton/pull/1400#issuecomment-403030776, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AhDfwGn9G_0Tqf5cFy3y9-mzTYDpUNLCks5uD2LhgaJpZM4UjEy2.

crd477 commented 6 years ago

PR submitted in #1454