On Thu, 30 Nov 2023 09:39:54 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

>> Can I please get a review of this change will attempts to workaround an 
>> issue in dsymutil?
>> 
>> The previous attempt to use `--reproducer Off` has shown that it fails to 
>> build on some other Xcode versions other than 14.3.1. Users have reported it 
>> to fail on  Xcode 15.0.1 and Xcode from a 12.4 devkit. The `--reproducer` 
>> option isn't being recognized on those versions.
>> 
>> This current PR proposes to use an alternate workaround, one which just sets 
>> an environment variable that Xcode 14.3.1 recognizes and prevents the 
>> creation of the leftover `dsymutil-*` temporary directories. Since this new 
>> approach uses an environment variable to workaround the issue, the command 
>> itself shouldn't ideally run into any usage issues. Once we agree upon this 
>> change, before integrating, I'll ask for inputs from those who had run into 
>> build issues to verify this alternate approach doesn't cause problems.
>> 
>> I have locally verified that this new approach continues to work and doesn't 
>> create those temporary directories. Additionally tier1, tier2, tier3 has 
>> completed successfully in our CI environment with this change.
>> 
>> Magnus, Erik, I looked around to see if there's any convention in setting 
>> such environment variables that will be used when invoking external tools 
>> during the build. I didn't find any similar usage. Although the current 
>> change in this PR is working, if there's a different and better way to do 
>> this, please do let me know.
>
> Jaikiran Pai has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - check for the support of --reproducer option before using it for dsymutil
>  - merge latest from master branch
>  - 8320931: [REDO] dsymutil command leaves around temporary directories

Looks good, passes SAP's build setup.

-------------

Marked as reviewed by clanger (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16876#pullrequestreview-1759737397

Reply via email to