Fix warning for unused lambda capture on Clang 9.1 #1400
Conversation
mbutrovich
requested review from
pmenon and
pervazea
and removed request for
pervazea
|
LGTM. @pervazea What is the status of getting the newer Mac onto the Jenkins infrastructure? |
|
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 <[email protected]>
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.
@pervazea<https://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 GitHub<#1400 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AhDfwGQZFv1zze9LM6UqwTVc40TLAbJrks5t79TXgaJpZM4UjEy2>.
|
|
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? |
|
@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? |
|
No. Section 2.B.iii of The software license agreement for macOS states: 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...] |
|
OK, computing services says:
I'm intend to make some progress on this today. |
|
The macOS VM has been added to the pool. Does the build stage that I specified for macOS look OK to you? |
|
Yes, macOS build stage looks ok.
Pervaze
…________________________________
From: Chad Dougherty <[email protected]>
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 macOS<http://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 GitHub<#1400 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AhDfwGn9G_0Tqf5cFy3y9-mzTYDpUNLCks5uD2LhgaJpZM4UjEy2>.
|
|
PR submitted in #1454 |
Fix warning for unused lambda capture on Clang 9.1
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.