Bug 51029
Summary: | Would like a pre-commit hook that checks for well-formedness of .vcproj/.vsprops XML files | ||
---|---|---|---|
Product: | WebKit | Reporter: | Adam Roben (:aroben) <aroben> |
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> |
Status: | NEW | ||
Severity: | Normal | CC: | abarth, ap, lforschler, mrowe, sfalken |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | All | ||
OS: | All |
Adam Roben (:aroben)
It's very common for people to introduce XML syntax errors when manually editing .vcproj/.vsprops files (see, e.g., <http://trac.webkit.org/changeset/73914> and <http://trac.webkit.org/changeset/73921>). It would be nice to have a pre-commit hook that would check that these files are well-formed. Steve suggested using `xmllint --noout`.
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Alexey Proskuryakov
Maybe this could be as early as svn-create-patch and/or style checker, in addition to commit hook.
William Siegrist
This is possible, but is a pre-commit hook the right solution? If we wanted pre-commit hooks to verify our syntax, why not have a tool that lints xml as well as C++, Python, IDL, etc. In other words, why does XML linting need to block commits but not C++?
Steve Falkenburg
Here are some recent build fixes required after incorrect edits to WebCore.vcproj that would have been caught by an XML well-formedness check:
http://trac.webkit.org/changeset/73887/trunk/WebCore/WebCore.vcproj/WebCore.vcproj
http://trac.webkit.org/changeset/73773/trunk/WebCore/WebCore.vcproj/WebCore.vcproj
http://trac.webkit.org/changeset/72795/trunk/WebCore/WebCore.vcproj/WebCore.vcproj
Adding this to the style checker might be an ok place to start.
This seems worse than a simple lint or style issue since these aren't stylistic problems (they're fatal errors).
Not sure where the line is between pre-commit hook and style checker. We check for tabs in pre-commit hook for example.
Adam Roben (:aroben)
Let's start with making check-webkit-style catch these errors, and then see if more is needed. I filed bug 51103.