Bug 206901 - [webkitperl] Clean up webkitdirs.pm
Summary: [webkitperl] Clean up webkitdirs.pm
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 231869
  Show dependency treegraph
 
Reported: 2020-01-28 12:06 PST by Basuke Suzuki
Modified: 2021-10-17 12:43 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Basuke Suzuki 2020-01-28 12:06:03 PST
If we are moving to python toolchain, no need to do that. But for many years, things have been untouched and it seems same in the near future. Let's start cleaning up the messy webkitdir.pm and modularlize them into webkitperl directory.
Comment 1 Basuke Suzuki 2020-01-28 12:27:26 PST
This is a meta bug for the goal.
Comment 2 Alexey Proskuryakov 2020-01-28 12:42:21 PST
This is going to be tough because of how webkitdirs.pm is used by into Apple internal scripts. But definitely a lot to improve here.
Comment 3 Basuke Suzuki 2020-02-03 12:15:02 PST
(In reply to Alexey Proskuryakov from comment #2)
> This is going to be tough because of how webkitdirs.pm is used by into Apple
> internal scripts. But definitely a lot to improve here.

Are there a lot of changes in Apple internal branch?
Comment 4 Jonathan Bedard 2020-02-03 14:46:35 PST
(In reply to Basuke Suzuki from comment #3)
> (In reply to Alexey Proskuryakov from comment #2)
> > This is going to be tough because of how webkitdirs.pm is used by into Apple
> > internal scripts. But definitely a lot to improve here.
> 
> Are there a lot of changes in Apple internal branch?

It not that there are possible conflicts, it's that there is code that might look dead in WebKit that's actually used by Apple internal scripts, or code that is used by Apple internal scripts in ways that the callee is not expecting.
Comment 5 Basuke Suzuki 2020-02-06 12:18:24 PST
(In reply to Jonathan Bedard from comment #4)
> (In reply to Basuke Suzuki from comment #3)
> > (In reply to Alexey Proskuryakov from comment #2)
> > > This is going to be tough because of how webkitdirs.pm is used by into Apple
> > > internal scripts. But definitely a lot to improve here.
> > 
> > Are there a lot of changes in Apple internal branch?
> 
> It not that there are possible conflicts, it's that there is code that might
> look dead in WebKit that's actually used by Apple internal scripts, or code
> that is used by Apple internal scripts in ways that the callee is not
> expecting.

Can Apple add unit test for those? Oh, wait. There's no unittest for webkitdirs.pm lol

So deleting unused function or changing interface of existing one will be dangerous, isn't it?

Also can EWS catch those internal build break?
Comment 6 Alexey Proskuryakov 2020-02-06 13:47:46 PST
> Also can EWS catch those internal build break?

No.
Comment 7 Basuke Suzuki 2020-02-07 11:37:38 PST
(In reply to Alexey Proskuryakov from comment #6)
> > Also can EWS catch those internal build break?
> 
> No.

That's so scary. Anyway, first target may be for CMake build related one.
Comment 8 Basuke Suzuki 2021-10-05 22:25:58 PDT
Goal:

- Modernize the codebase to 2021 perl syntax conventions
- Modularize by features.
- Modularize by platform.
- Stop using ad-hoc command line arguments. Document them well.
  (detect misspelled option)
- Passing cmake arguments easily. (i.e. arguments after "--" will be passed to cmake. Tools/Scripts/build-webkit --wincairo -- -DUSE_SYSTEM_MALLOC=Off)

Currently the last one is already implemented, but it conflicts with the other goal, "stop using ad-hoc command" because there's no clear separation to options to cmake args. Using "--" is a new thing and a good sign to introduce new features, I think.
Comment 9 Basuke Suzuki 2021-10-05 22:49:29 PDT
Something like this:

1. Tools/Scripts/build-webkit --unknown-option

Run as same as current. Maybe display warnings in the future

2. Tools/Scripts/build-webkit -DSOME_CMAKE_VAR=Off

Run as same as current. Will be added to cmake args. Maybe warnings in the future.

3. Tools/Scripts/build-webkit --unknown-option --

Error. Unknown option "--unknown-option"

3. Tools/Scripts/build-webkit --wincairo -- -DUSE_SYSTEM_MALLOC=Off

Option --wincairo is used and "-DUSE_SYSTEM_MALLOC=Off" will be passed to cmake on top of the generated arguments in the script.