[presubmit] Enforce language attribute in project.yaml to be always set. #3477

Dor1s · 2020-03-06T20:52:29Z

My original intent was to enforce this for non C/C++ projects, but I couldn't come up with a good way to do it only in such cases. Landing this change might somewhat annoy people who contribute more changes to OSS-Fuzz for already integrated C/C++ projects, but that's fine with me -- yet another reason to remind people of the ideal integration.

UPD: when landing this change, I'll update all existing project.yaml files and we'll enforce the attribute from now on.

TravisBuddy · 2020-03-06T20:59:55Z

Hey @Dor1s,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 6d85baf0-5fed-11ea-bf9c-5be26ced5bf7

inferno-chromium · 2020-03-06T21:26:49Z

infra/presubmit.py

@@ -190,12 +190,10 @@ def check_valid_emails(self):
  def check_valid_language(self):
    """Check that the language specified is valid."""
    language = self.data.get('language')
-    if not language:
-      return
-
    if language not in self.LANGUAGES_SUPPORTED:


Maybe add a better message when language is None
like "Missing language attribute" ?
Also, does our documentation for C/C++ mention the language attribute in template ?

Also, does our documentation for C/C++ mention the language attribute in template ?

Nope, and I also need to test this before landing, if we agree that's an acceptable solution.

jonathanmetzman

When i tested, it appears that language: c++ is interpreted by pyyaml to be {'language': 'c++'}, so we don't need cpp.

I wouldn't say im opposed to this, but it seems like a blunt solution for the issue of coverage builds happening for non-C++ projects (I don't know enough about the coverage infra to think of an alternative).
I wonder if people in ambiguous language cases (e.g. Python libraries written in C) would get confused.

jonathanmetzman · 2020-03-06T23:34:09Z

I wouldn't say im opposed to this, but it seems like a blunt solution for the issue of coverage builds happening for non-C++ projects (I don't know enough about the coverage infra to think of an alternative).
Offline max clarified that there are other reasons to do this, srcmap support is different for example, so this looks like a better solution than I initially thought.

Dor1s · 2020-03-06T23:36:55Z

Yeah, there are more use cases mentioned in #3297:

different coverage logic
different srcmap implementation
default build configs for different languages
$LIB_FUZZING_ENGINE_DEPRECATED vs $LIB_FUZZING_ENGINE, e.g. for Rust we can provide the deprecated version as a non-deprecated one since that's the intended way to use it
(more speculative) bad build checks can differ for other languages

Dor1s · 2020-03-06T23:37:32Z

When i tested, it appears that language: c++ is interpreted by pyyaml to be {'language': 'c++'}, so we don't need cpp.

I'll change cpp to c++ and also update all existing projects so that the new presubmit check doesn't annoy users who will be making unrelated changes in OSS-Fuzz repo after we land this.

jonathanmetzman

LGTM

Dor1s

I've updated the documentation, presubmit check, and the new project template used by helper.py.

Please take a look. If this looks good, I'll update all the projects with missing attribute and land the change.

Dor1s · 2020-03-09T17:04:33Z

infra/presubmit.py

@@ -190,12 +190,10 @@ def check_valid_emails(self):
  def check_valid_language(self):
    """Check that the language specified is valid."""
    language = self.data.get('language')
-    if not language:
-      return
-
    if language not in self.LANGUAGES_SUPPORTED:


TravisBuddy · 2020-03-09T17:11:53Z

Travis tests have failed

Hey @Dor1s,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: 11123ad0-6229-11ea-ba95-012c7b325999

TravisBuddy · 2020-03-09T21:34:52Z

Hey @Dor1s,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: ce24e630-624d-11ea-ba95-012c7b325999

Dor1s · 2020-03-09T21:37:44Z

@oliverchang PTAL too, maybe you have another idea or any concerns regarding this one.

oliverchang

LGTM

oliverchang · 2020-03-10T01:47:50Z

infra/templates.py


 PROJECT_YAML_TEMPLATE = """\
 homepage: "<your_project_homepage>"
+language: <programming_language>


nit: give valid values as examples, as otherwise it's not clear what's expected (e.g. cpp or c++) ?

TravisBuddy · 2020-03-10T17:50:47Z

Travis tests have failed

Hey @Dor1s,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: aa2f4450-62f7-11ea-b4e6-7500f47047b3

…set.

…ue for now

Dor1s · 2020-03-10T18:07:11Z

I've added labels and selective_unpack sections to presubmit script plus fixed a few files manually. There's still a dozen of presubmit errors (for missing primary_contact, non fuzzing engine (e.g. chakra), and some other custom value used only once), which I can't fix, so will land with them.

TravisBuddy · 2020-03-10T19:57:53Z

Hey @Dor1s,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 6c4f1900-6309-11ea-8dd1-c14dbc1d5bef

… runner. Should've done this in #3477. Good news is that nothing failed, the coverage job was just skipped for pretty much all projects.

…s. (#3493) * [infra] Change language attribute from "cpp" to "c++" in coverage job runner. Should've done this in #3477. Good news is that nothing failed, the coverage job was just skipped for pretty much all projects. * also remove default value from build_project script

Dor1s requested review from oliverchang and jonathanmetzman March 6, 2020 20:52

Dor1s changed the title ~~[presubmit] Enforce language attribute in projectt.yaml to be always set.~~ [presubmit] Enforce language attribute in project.yaml to be always set. Mar 6, 2020

inferno-chromium reviewed Mar 6, 2020

View reviewed changes

jonathanmetzman reviewed Mar 6, 2020

View reviewed changes

jonathanmetzman approved these changes Mar 6, 2020

View reviewed changes

Dor1s commented Mar 9, 2020

View reviewed changes

oliverchang approved these changes Mar 10, 2020

View reviewed changes

Dor1s added 9 commits March 10, 2020 11:03

[presubmit] Enforce language attribute in projectt.yaml to be always …

40350ed

…set.

Update documentation, better presubmit check, new project template.

bc48357

add docstring to templates.py

63f7484

Add example values in the project.yaml template and remove python val…

a54540e

…ue for now

Add "project: c++" to 256 projects

a335aa7

format

5b1322e

Add labels and selective_unpack sections to the presubmit check

544bc1d

fix incorrect auto_ccs format in three projects

7321f83

fix nss emails after rebase

7c69562

Dor1s force-pushed the language branch from f60e725 to 7c69562 Compare March 10, 2020 18:05

Dor1s merged commit 71f4914 into master Mar 10, 2020

Dor1s added a commit that referenced this pull request Mar 12, 2020

[infra] Change language attribute from "cpp" to "c++" in coverage job…

cc0ba37

… runner. Should've done this in #3477. Good news is that nothing failed, the coverage job was just skipped for pretty much all projects.

Dor1s mentioned this pull request Mar 12, 2020

[infra] Change language attribute from "cpp" to "c++" in build scripts. #3493

Merged

Dor1s mentioned this pull request Mar 27, 2020

Adds project suricata #2533

Merged

jonathanmetzman deleted the language branch November 19, 2020 19:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[presubmit] Enforce language attribute in project.yaml to be always set. #3477

[presubmit] Enforce language attribute in project.yaml to be always set. #3477

Dor1s commented Mar 6, 2020 •

edited

Loading

TravisBuddy commented Mar 6, 2020

inferno-chromium Mar 6, 2020

Dor1s Mar 6, 2020

Dor1s Mar 9, 2020

jonathanmetzman left a comment

jonathanmetzman commented Mar 6, 2020

Dor1s commented Mar 6, 2020

Dor1s commented Mar 6, 2020

jonathanmetzman left a comment

Dor1s left a comment

Dor1s Mar 9, 2020

TravisBuddy commented Mar 9, 2020

TravisBuddy commented Mar 9, 2020

Dor1s commented Mar 9, 2020

oliverchang left a comment

oliverchang Mar 10, 2020

TravisBuddy commented Mar 10, 2020

Dor1s commented Mar 10, 2020

TravisBuddy commented Mar 10, 2020

[presubmit] Enforce language attribute in project.yaml to be always set. #3477

[presubmit] Enforce language attribute in project.yaml to be always set. #3477

Conversation

Dor1s commented Mar 6, 2020 • edited Loading

TravisBuddy commented Mar 6, 2020

TravisBuddy Request Identifier: 6d85baf0-5fed-11ea-bf9c-5be26ced5bf7

inferno-chromium Mar 6, 2020

Choose a reason for hiding this comment

Dor1s Mar 6, 2020

Choose a reason for hiding this comment

Dor1s Mar 9, 2020

Choose a reason for hiding this comment

jonathanmetzman left a comment

Choose a reason for hiding this comment

jonathanmetzman commented Mar 6, 2020

Dor1s commented Mar 6, 2020

Dor1s commented Mar 6, 2020

jonathanmetzman left a comment

Choose a reason for hiding this comment

Dor1s left a comment

Choose a reason for hiding this comment

Dor1s Mar 9, 2020

Choose a reason for hiding this comment

TravisBuddy commented Mar 9, 2020

Travis tests have failed

TravisBuddy Request Identifier: 11123ad0-6229-11ea-ba95-012c7b325999

TravisBuddy commented Mar 9, 2020

TravisBuddy Request Identifier: ce24e630-624d-11ea-ba95-012c7b325999

Dor1s commented Mar 9, 2020

oliverchang left a comment

Choose a reason for hiding this comment

oliverchang Mar 10, 2020

Choose a reason for hiding this comment

TravisBuddy commented Mar 10, 2020

Travis tests have failed

TravisBuddy Request Identifier: aa2f4450-62f7-11ea-b4e6-7500f47047b3

Dor1s commented Mar 10, 2020

TravisBuddy commented Mar 10, 2020

TravisBuddy Request Identifier: 6c4f1900-6309-11ea-8dd1-c14dbc1d5bef

Dor1s commented Mar 6, 2020 •

edited

Loading