WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 208514
[ews] Add build step to find list of modified changelogs
https://bugs.webkit.org/show_bug.cgi?id=208514
Summary
[ews] Add build step to find list of modified changelogs
Aakash Jain
Reported
2020-03-03 07:48:19 PST
Commit queue needs to generate a commit message (from the ChangeLog files). For this it needs the find the list of modified ChangeLog files.
Attachments
Patch
(8.70 KB, patch)
2020-03-03 07:55 PST
,
Aakash Jain
no flags
Details
Formatted Diff
Diff
Patch
(9.51 KB, patch)
2020-03-03 10:18 PST
,
Aakash Jain
jbedard
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Aakash Jain
Comment 1
2020-03-03 07:55:14 PST
Created
attachment 392274
[details]
Patch
Aakash Jain
Comment 2
2020-03-03 08:23:57 PST
Comment on
attachment 392274
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=392274&action=review
> Tools/BuildSlaveSupport/ews-build/steps.py:2449 > + def _status_regexp(self, expected_types):
This comes from
https://trac.webkit.org/browser/webkit/trunk/Tools/Scripts/webkitpy/common/checkout/scm/git.py#L176
Aakash Jain
Comment 3
2020-03-03 08:27:30 PST
Sample runs:
https://ews-build.webkit-uat.org/#/builders/26/builds/1042
https://ews-build.webkit-uat.org/#/builders/26/builds/1041
https://ews-build.webkit-uat.org/#/builders/26/builds/1040
Jonathan Bedard
Comment 4
2020-03-03 08:42:54 PST
Comment on
attachment 392274
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=392274&action=review
> Tools/BuildSlaveSupport/ews-build/steps.py:2423 > + command = ['git', 'diff', '-r', '--name-status', '--no-renames', '--no-ext-diff', '--full-index']
Commit queue uses git? That surprises me. I would expect it to be native svn.
> Tools/BuildSlaveSupport/ews-build/steps.py:2442 > + modified_changelogs = self.extract_changelogs(log_text, self._status_regexp('ADM'))
Do we ever expect this to not be 'Added, Deleted, Modified? Might be better to just create and compile the regex ahead of time.
> Tools/BuildSlaveSupport/ews-build/steps_unittest.py:3035 > +M Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp''') +
Test added and deleted?
Aakash Jain
Comment 5
2020-03-03 10:18:03 PST
Created
attachment 392290
[details]
Patch
Aakash Jain
Comment 6
2020-03-03 10:19:33 PST
View in context:
https://bugs.webkit.org/attachment.cgi?id=392274&action=review
>> Tools/BuildSlaveSupport/ews-build/steps.py:2423 >> + command = ['git', 'diff', '-r', '--name-status', '--no-renames', '--no-ext-diff', '--full-index'] > > Commit queue uses git? That surprises me. I would expect it to be native svn.
Yes, all EWS bots (including commit-queue on old ews) use git. e.g.:
https://bugs.webkit.org/show_bug.cgi?id=207727#c10
>> Tools/BuildSlaveSupport/ews-build/steps.py:2442 >> + modified_changelogs = self.extract_changelogs(log_text, self._status_regexp('ADM')) > > Do we ever expect this to not be 'Added, Deleted, Modified? Might be better to just create and compile the regex ahead of time.
compiling wouldn't change much, this is very simple regex, and used only once per build.
>> Tools/BuildSlaveSupport/ews-build/steps_unittest.py:3035 >> +M Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp''') + > > Test added and deleted?
Added unit-tests for ChangeLog addition. We don't need to consider ChangeLog deletion (otherwise the next step of generating commit message will fail for deleted files). Updated steps.py to look at only addition and modification.
Aakash Jain
Comment 7
2020-03-03 12:08:15 PST
Committed
r257794
: <
https://trac.webkit.org/changeset/257794
>
Radar WebKit Bug Importer
Comment 8
2020-03-03 12:09:32 PST
<
rdar://problem/60007220
>
Aakash Jain
Comment 9
2020-03-04 08:10:02 PST
Just for reference, output from old commit-queue when the patch doesn't have any ChangeLog (pretty unreadable):
https://bugs.webkit.org/show_bug.cgi?id=208495#c10
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug