eclipse-equinox / equinox

equinox
Eclipse Public License 2.0
28 stars 60 forks source link

Fix warning message on Mac about secure coding not enabled #632

Open Phillipus opened 1 month ago

Phillipus commented 1 month ago
github-actions[bot] commented 1 month ago

Test Results

  660 files  +  220    660 suites  +220   1h 11m 18s :stopwatch: + 21m 49s 2 195 tests ±    0  2 148 :white_check_mark: +    1   47 :zzz:  -  1  0 :x: ±0  6 729 runs  +2 243  6 586 :white_check_mark: +2 196  143 :zzz: +47  0 :x: ±0 

Results for commit d15cb066. ± Comparison against base commit c4be7f2d.

HannesWell commented 1 month ago

@sravanlakkimsetti can you review this?

@Phillipus thank you for being pro-active on this topic. You can try out the executables/launcher-libraries archived in the Jenkins verification build to test if it has the desired effect.

Phillipus commented 1 month ago

@Phillipus thank you for being pro-active on this topic. You can try out the executables/launcher-libraries archived in the Jenkins verification build to test if it has the desired effect.

@HannesWell Thanks for the heads up. I had already compiled and built the so file locally and tested it. But I also tested the one you refer to at Jenkins here. That does indeed have the desired effect.

Phillipus commented 1 month ago

I'm not certain that setting an app delegate where I have set it in the PR is the right place. There are places that reference a an existing delegate:

https://github.com/eclipse-equinox/equinox/blob/c4be7f2d1bfab28d171e7ead3a95632a924505cf/features/org.eclipse.equinox.executable.feature/library/cocoa/eclipseCocoa.c#L197-L198

https://github.com/eclipse-equinox/equinox/blob/c4be7f2d1bfab28d171e7ead3a95632a924505cf/features/org.eclipse.equinox.executable.feature/library/cocoa/eclipseCocoa.c#L211-L212

https://github.com/eclipse-equinox/equinox/blob/c4be7f2d1bfab28d171e7ead3a95632a924505cf/features/org.eclipse.equinox.executable.feature/library/cocoa/eclipseCocoa.c#L234-L235

I'm not sure if setting a new delegate where I have might over-write a possibly existing one and then those functions might not work.

Phillipus commented 1 month ago

I did a check, and at the point where this PR sets the AppDelegate it is NULL but I'm not sure where or if an AppDelegate is set somewhere else. @lshanmug can you advise?

sravanlakkimsetti commented 1 month ago

@subyssurendran666 Can you please take a look?

Lets merge this after 4.32

Phillipus commented 1 month ago

Don't merge this one without further investigation. The equivalent implementation in SWT caused a crash to desktop. See https://github.com/eclipse-platform/eclipse.platform.swt/pull/1240