On Tue, 27 Feb 2024 10:32:24 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:

>>> Oh you are right, we should always add the OUTPUTDIR to both maps 
>>> regardless of if it's internal to the WORKSPACE_ROOT or not. Paths relative 
>>> to the OUTPUTDIR should be stable across builds in different output 
>>> directories, but paths inside the OUTPUTDIR, expressed as relative to the 
>>> WORKSPACE_ROOT will vary.
>> 
>> @erikj79 @magicus Yes, it needs to always be present as you've pointed out 
>> the OUTPUTDIR when within the WORKSPACE will be 
>> eg.WORKSPACE/build/linux-aarch64-server-release/support and the standard 
>> WORKSPACE mapping will make that build/linux-aarch64-server-release/support, 
>> but a build dir outside will be simply <any_path>/support
>
> It seems correct to include the output dir in the remapping, but I have two 
> objections/questions to the way you are doing it.
> 1) Why not just use OUTPUTDIR instead of the two specialized subdirs? That is 
> simpler, more general and future-proof.
> 2) Why not expand the value of the OUTPUTDIR variable? I.e.  
> 
>  DEBUG_PREFIX_CFLAGS="$DEBUG_PREFIX_CFLAGS -fdebug-prefix-map=$OUTPUTDIR/="
>  ```
> 
> instead of trying to preserve it as a variable to be expanded in the make 
> execution.

Also, this mapping business is getting really convoluted. :-( I did not like it 
as it was, and this patch makes it even worse. I think we need to rewrite this 
to create some kind of dict/map, and then iterate over the map and apply 
`-fdebug-prefix-map` to all items in the map. Unfortunately, the data 
structures available in shell scripts is limited and will make this a bit 
tricky to pull off. :(

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18009#discussion_r1504011145

Reply via email to