Add a `--dev` option to `mach build` and require either `--dev` or `--release`

[jinankjain]

[highfive]

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @jdm (or someone else) soon.

[ghost]

@jdm @mbrubeck as you suggested in #5941 I have implemented the third option that whenever a user build using --debug then then only warning is shown if both release and debug not set in that it exits compilation with a warning

[mbrubeck]

^{python/servo/build_commands.py, line 73 [r1] (raw file):}
We should update the build instructions in the README.md file to include this flag.

---

^{python/servo/build_commands.py, line 102 [r1] (raw file):}
We should also exit with an error if release and debug are both True.

Nit: Python style convention is to omit the outer parentheses, and add a space after not.

---

^{python/servo/build_commands.py, line 103 [r1] (raw file):}
If neither flag is True, then we should look for a default mode in self.config (which contains options from the parsed .servobuild file). If there's no flag and no default mode, then we should display an error.

The error should explain exactly what to do, and (briefly) what the difference is. For example: "Please specify either --debug for a development build or --release for an optimized build."

---

^{python/servo/build_commands.py, line 162 [r1] (raw file):}
I think this warning is no longer needed; we'll rely on the error message above instead. That way we won't annoy deveolpers who frequently build in debug mode.

---

^{servobuild.example, line 27 [r1] (raw file):}
I'd suggested changing this to a mode option, so it can allow both mode="debug" and mode ="release" (and any other profiles that Cargo adds in the future.

---

Comments from the review on Reviewable.io

[mbrubeck]

Thanks! I left a few comments on Reviewable, above.

[ghost]

@mbrubeck Could you please review the new full request I have amended the changes as you have suggested

[mbrubeck]

Reviewed files:

• python/servo/build_commands.py @ r1, r2
• README.md @ r2
• servobuild.example @ r1

---

^{python/servo/build_commands.py, line 73 [r2] (raw file):}
Sorry I overlooked this before, but we should actually use --dev instead of --debug for consistency with other commands (like mach run).

---

Comments from the review on Reviewable.io

[mbrubeck]

Reviewed files:

• python/servo/build_commands.py @ r2
• README.md @ r2
• servobuild.example @ r1

---

^{python/servo/build_commands.py, line 103 [r1] (raw file):}
The first half of this comment (about self.config) still needs to be addressed.

---

Comments from the review on Reviewable.io

[mbrubeck]

@jinankjain Thanks! That addresses most of the issues. There are still a couple of issues that haven't been addressed—in particular, reading the default from the config file. Also, I added one more issue that I missed previously. See the comments on Reviewable for details.

[ghost]

@mbrubeck I did not get the thing with config file what exactly needs to be done

[ghost]

@mbrubeck I had done that patch with --debug to --dev you could review that in my new PR

[mbrubeck]

@jinankjain: To read an option from the servobuild config file, you need to look in the self.config dictionary. For example, see the code to check the android setting in .servobuild.

If both dev and release are false, then mach build should check self.config["build"]["mode"]. If it is set to "dev" then set the dev flag to True, and if it is set to "release" then set the release option to True.

The self.config dictionary is populated in the CommandBase constructor. You can add code there to set config["build"]["mode"] to a default value of "", similar to this code which sets config["build"]["android"] to a default value of False.

[ghost]

@mbrubeck you could review the new PR

[mbrubeck]

^{servobuild.example, line 28 [r3] (raw file):}
Nit: "dev" needs to be in quotation marks (mode="dev").

---

Comments from the review on Reviewable.io

[mbrubeck]

Reviewed files:

• python/servo/build_commands.py @ r3
• python/servo/command_base.py @ r3
• README.md @ r3
• servobuild.example @ r4

---

Comments from the review on Reviewable.io

[ghost]

Ya I fixed that up

[mbrubeck]

Everything looks great. Thank you!

Note: We will need to deploy a change to our test infrastructure before we can merge this. It'll be a little tricky because the two chances need to land at the same time. I'll work on this next week. See servo/saltfs#28 for details.

[ghost]

Any updates @mbrubeck ?

[mbrubeck]

This can land after #6023 and servo/saltfs#28 are merged and deployed. (It will need to be rebased, since part of it was split into #6023.)

[bors-servo]

☔ The latest upstream changes (presumably #6023) made this pull request unmergeable. Please resolve the merge conflicts.

[ghost]

What are merge conflicts should I have to look into it @mbrubeck ?

[mbrubeck]

I pushed the changes to add the --dev CommandOption, but nothing else. You can rebase your patch onto master, and just leave out the parts that already landed in #6023. Or I can rebase and push this if you would like.

[ghost]

Ya you could rebase and push it :)

[mbrubeck]

Rebased and moved to #6038.

[mbrubeck]

This was merged in the other PR. Thank you @jinankjain!