Bug 221621 – [Cocoa] rename `ENGINEERING_BUILD` to `ENABLE_DEVELOPER_MODE` to match other platforms

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Devin Rousso]

see <https://bugs.webkit.org/show_bug.cgi?id=221371#c16>

[Devin Rousso]

Created attachment 419749 [details]
Patch

[Alexey Proskuryakov]

It's great to rename it from ENGINEERING_BUILD, but I am still concerned about introducing this concept. It's not OK to have different code between Release and Production. It's not OK to assume that local builds are Release, and CI builds are Production (they often aren't).

[Devin Rousso]

(In reply to Alexey Proskuryakov from comment #2)
> It's great to rename it from ENGINEERING_BUILD, but I am still concerned about introducing this concept. It's not OK to have different code between Release and Production. It's not OK to assume that local builds are Release, and CI builds are Production (they often aren't).

It sounds like you don't want this to land then?  IMO there's a huge amount of utility in being able to have minor tweaks that make debugging easier for local builds (e.g. giving a name to the injected media controls scripts so it's visible in Web Inspector).  I'm not trying to "shame" you or change your mind or anything like that, I'm just asking because if this isn't going to land then we should land Bug 221613 in the meantime (or perhaps remove `ENGINEERING_BUILD` altogether) so non-Xcode builds aren't blocked.

FYI WebInspectorUI has been doing something similar to this for years with `COMBINE_INSPECTOR_RESOURCES`/`COMBINE_TEST_RESOURCES` and AFAIK there haven't been any issues there.

[Devin Rousso]

Comment on attachment 419749 [details]
Patch

Seeing as `ENGINEERING_BUILD` is already in trunk and this patch just renames it so that non-Xcode builds aren't broken, I'm going to land this.

I'm not doing this as any sort of "put my foot down and say we're keeping it" kind of thing. I'm more than willing to revisit the discussion (and even remove it if that's the agreed upon result).  I just want to unblock non-Xcode builds.

[EWS]

Committed r272617: <https://commits.webkit.org/r272617>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 419749 [details].

[Alexey Proskuryakov]

We should remove ENGINEERING_BUILD altogether, and find a different way to achieve the goal. It's not doing quite what you want it to do, and it's going to get farther away from what's needed in the future.

[Michael Catanzaro]

I don't think it will ever be possible to remove DEVELOPER_MODE from other ports.

E.g. it's used in CMake to decide whether to hide internal symbols, which can't be done in development builds since that would prevent linking API tests, layout test runner, etc.

[Alexey Proskuryakov]

I am not concerned about other ports. It is the invalid distinction between Debug/Release and production builds on Apple platforms that should not exist. 

Devin, I’m going to post a patch to remove it tomorrow if you don’t beat me to it. We can think about better ways to achieve the goal separately.

[Radar WebKit Bug Importer]

<rdar://problem/74206419>