Skip to content

/ saltfs

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

Add an env variable to the release builder to tell mach to build with debug assertions. #480

Merged
merged 1 commit into from Sep 16, 2016
Merged

Add an env variable to the release builder to tell mach to build with debug assertions. #480

merged 1 commit into from Sep 16, 2016

Conversation

@emilio
Copy link
Member

emilio commented Sep 16, 2016 •
edited by larsbergstrom

r? @aneeshusa

This change is Reviewable
@aneeshusa
aneeshusa suggested changes Sep 16, 2016
View changes
buildbot/master/files/config/master.cfg Outdated
@@ -170,7 +170,7 @@ c['builders'] = [
DynamicServoBuilder("arm64", CROSS_SLAVES, envs.build_arm64),
DynamicServoBuilder("linux-dev", LINUX_SLAVES, envs.build_linux),
DynamicServoBuilder("linux-nightly", LINUX_SLAVES, envs.build_linux),
DynamicServoBuilder("linux-rel", LINUX_SLAVES, envs.build_linux),
DynamicServoBuilder("linux-rel", LINUX_SLAVES, envs.build_linux_rel_debug_assert),

This comment has been minimized.

Sign in to view
@aneeshusa

aneeshusa Sep 16, 2016

Member

You'll need to move the last argument to the next line to fit inside the line length limit.

This comment has been minimized.

Sign in to view
@emilio

emilio Sep 16, 2016

Author Member

Gah, good catch, thanks :)

Done
@emilio emilio force-pushed the emilio:debug_assertions branch from 07399ae to 61a3efa Sep 16, 2016
@aneeshusa
aneeshusa suggested changes Sep 16, 2016
View changes
buildbot/master/files/config/master.cfg Outdated
@@ -170,7 +170,8 @@ c['builders'] = [
DynamicServoBuilder("arm64", CROSS_SLAVES, envs.build_arm64),
DynamicServoBuilder("linux-dev", LINUX_SLAVES, envs.build_linux),
DynamicServoBuilder("linux-nightly", LINUX_SLAVES, envs.build_linux),
DynamicServoBuilder("linux-rel", LINUX_SLAVES, envs.build_linux),
DynamicServoBuilder("linux-rel", LINUX_SLAVES

This comment has been minimized.

Sign in to view
@aneeshusa

aneeshusa Sep 16, 2016

Member

You'll need a comma after this.

This comment has been minimized.

Sign in to view
@emilio

emilio Sep 16, 2016

Author Member

Pffft... Thanks, done, should learn to be more careful when pushing fixups, sigh.

Are there docs in how to install the required deps and test changes to this file, btw?

This comment has been minimized.

Sign in to view
@aneeshusa

aneeshusa Sep 16, 2016

Member

It's partially documented right now (the Vagrant-related sections on the wiki), but the test suite isn't well documented right now. I have a half written draft sitting on my hard drive somewhere...
@emilio
Add an environment variable to tell mach to build with debug assertions.
Unverified
This user has not uploaded their public key yet.
GPG key ID: 056B727BB9C1027C Learn about signing commits
Loading status checks…
97d8768 
Part of servo/servo#13127
@emilio emilio force-pushed the emilio:debug_assertions branch from 61a3efa to 97d8768 Sep 16, 2016
@aneeshusa
aneeshusa approved these changes Sep 16, 2016
View changes
@aneeshusa
Copy link
Member

aneeshusa commented Sep 16, 2016

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Sep 16, 2016

📌 Commit 97d8768 has been approved by aneeshusa
@bors-servo
Copy link
Contributor

bors-servo commented Sep 16, 2016

⌛ Testing commit 97d8768 with merge 9ea333b...
bors-servo added a commit that referenced this pull request Sep 16, 2016
@bors-servo
Auto merge of #480 - emilio:debug_assertions, r=aneeshusa
Loading status checks…
9ea333b 
Add an env variable to the release builder to tell mach to build with debug assertions.

r? @aneeshusa

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/480)
<!-- Reviewable:end -->
@aneeshusa aneeshusa self-assigned this Sep 16, 2016
@aneeshusa aneeshusa mentioned this pull request Sep 16, 2016
Closed
@bors-servo
Copy link
Contributor

bors-servo commented Sep 16, 2016

☀️ Test successful - status-travis
@bors-servo bors-servo merged commit 97d8768 into servo:master Sep 16, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@jdm
Copy link
Member

jdm commented Sep 16, 2016

I'm a bit nervous that whenever we next update the builders to use this we'll have permafailures that we've never seen before...
@emilio emilio deleted the emilio:debug_assertions branch Sep 16, 2016
@emilio
Copy link
Member Author

emilio commented Sep 16, 2016

@jdm: The way we're doing this is effectively to avoid it. This env var currently does nothing.

Once this is rolled out, I need to add a mach PR using that env var to add the --cfg flag to the rust flags, and then cry with the failures, and fix it, or figure out what we can do about it :)
Coder206 added a commit to Coder206/saltfs that referenced this pull request Apr 22, 2017
Rohit Burra @Coder206
Remove enviroment variables for debug assertions (reverts servo#480)
c32dc6a 
This reverts commit 97d8768. Follow up
to #14025 servo issue. Adds linux-rel-wpt and linux-rel-css in place of
linux-rel
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aneeshusa aneeshusa

Assignees

@aneeshusa aneeshusa

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@emilio @aneeshusa @bors-servo @jdm
You can’t perform that action at this time.