Bug 229418 – Build error preprocessing sandbox

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Per Arne Vollan]

In some cases there are build errors when preprocessing the WebContent sandbox.

[Per Arne Vollan]

<rdar://81480311>

[Per Arne Vollan]

Created attachment 436231 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 436231 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=436231&action=review

> Source/WebKit/Scripts/generate-derived-sources.sh:12
> +if [ ! -z ${JAVASCRIPTCORE_PRIVATE_HEADERS_DIR} ]; then

-z is "the length of string is zero", can you elaborate on the logic of this check?

I expected this to check for sandbox-profiles-ios argument that's passed when building the profiles.

[Per Arne Vollan]

(In reply to Alexey Proskuryakov from comment #3)
> Comment on attachment 436231 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=436231&action=review
> 
> > Source/WebKit/Scripts/generate-derived-sources.sh:12
> > +if [ ! -z ${JAVASCRIPTCORE_PRIVATE_HEADERS_DIR} ]; then
> 
> -z is "the length of string is zero", can you elaborate on the logic of this
> check?
>

Yes, this checks that the env variable exists (does not have zero length) before using it. If the env variable does not exist, the ln command will fail. 
 
> I expected this to check for sandbox-profiles-ios argument that's passed
> when building the profiles.

I think the above test will cover more cases, in case this should happen under other circumstances. Also, it will continue to work if we should rename the argument.

Thanks for reviewing!

[Alexey Proskuryakov]

My concern is that we will get a way more confusing error from make if this happens under other circumstances. 

So I'd prefer it to be as focused as possible, so that the check is only true when we know it should be true - not when it happens to be true.

[Per Arne Vollan]

(In reply to Alexey Proskuryakov from comment #5)
> My concern is that we will get a way more confusing error from make if this
> happens under other circumstances. 
> 
> So I'd prefer it to be as focused as possible, so that the check is only
> true when we know it should be true - not when it happens to be true.

That makes sense, I will update the patch.

Thanks for reviewing!

[Per Arne Vollan]

Created attachment 436301 [details]
Patch

[Per Arne Vollan]

Comment on attachment 436301 [details]
Patch

Thanks for reviewing!

[EWS]

Committed r281577 (240941@main): <https://commits.webkit.org/240941@main>

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

[Per Arne Vollan]

Reopening to attach new patch.

[Per Arne Vollan]

Created attachment 436428 [details]
Patch

[Per Arne Vollan]

Comment on attachment 436428 [details]
Patch

Thanks for reviewing!

[EWS]

Committed r281595 (240957@main): <https://commits.webkit.org/240957@main>

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