RFR: 8046765 : (s) makefiles should use parameterized $(CP) and $(MV) rather than explicit commands

[Vladimir Kozlov]

Hi, Mike

Changes looks good to me too. Thank you for cleaning up trailing spaces.
I verified that my local Hotspot build on Solaris works with your patch.

Thanks,
Vladimir

On 7/14/14 4:00 PM, David Holmes wrote:
> Hi Mike,
>
> On 15/07/2014 3:11 AM, Mike Duigou wrote:
>>
>> On Jul 13 2014, at 23:43 , David Holmes <David.Holmes at oracle.com> wrote:
>>
>>> Hi Mike,
>>>
>>> The changes from cp to $(CP) look fine, as do the couple of mv
>>> changes. As I think I said first time round I'm not sure why cp and
>>> mv are being singled out here
>>
>> The static analysis tool we are using substitutes instrumented
>> versions of mv and cp so that it can track files from their final
>> location back to the source. It would seem that hashing would be a
>> more reliable way to do this tracking but this is what the tool requires.
>
> Ah I see.
>
>>> (and I note that windows also does $(RM) but the others don't).
>>
>> $RM was already defined in build.make. I don't see any changes in my
>> patch involving RM. I do note that the RM expansion isn't used in most
>> makefiles (sa.make was the one I noticed).
>>
>>>
>>> There seem to be a lot of spurious changes in the patch file that
>>> don't show up in the cdiffs. I assume whitespace has been added,
>>> which jcheck will reject (whitespace can't have been removed as
>>> jcheck would not have permitted it in the first place).
>>
>> It is actually whitespace being removed as a consequence of my text
>> editor trimming trailing whitespace. jcheck whitespace checks only
>> specific file types (java|c|h|cpp|hpp)
>
> I didn't realize that. I suppose the tab checking in makefiles would be
> a bit tricky :)
>
>>>
>>> Also please update all the Oracle copyright lines as needed (hotspot
>>> policy).
>>
>> I shall do so.
>>
>> With the copyright changes are we good to go?
>
> Absolutely from my perspective. But you need a second reviewer if not
> already present.
>
> Thanks,
> David
>
>
>
>
>
>>>
>>> Thanks,
>>> David
>>>
>>> On 13/07/2014 3:19 AM, Mike Duigou wrote:
>>>> Hello all;
>>>>
>>>> After further testing some additional changes were found to be
>>>> needed to support building hotspot without configure support. There
>>>> are a small number of additional changes in various buildtree.make
>>>> and the windows build.make files to ensure that locations are
>>>> defined for CP and MV commands. If there's a more appropriate
>>>> location for these definitions please suggest it.
>>>>
>>>> The patch is otherwise unchanged from the month ago version except
>>>> for line number offset changes owing to other intervening changesets.
>>>>
>>>> jbsbug: https://bugs.openjdk.java.net/browse/JDK-8046765
>>>> webrev: http://cr.openjdk.java.net/~mduigou/JDK-8046765/2/webrev/
>>>>
>>>> Thanks!
>>>>
>>>> Mike
>>>>
>>>> On Jun 13 2014, at 12:43 , Mike Duigou <mike.duigou at oracle.com> wrote:
>>>>
>>>>> Hello all;
>>>>>
>>>>> This is a small changeset to the hotspot makefiles to have them use
>>>>> expansions of the $(CP) and $(MV) variables rather than explicit
>>>>> commands for all operations involving files in the deliverables.
>>>>> This changes is needed by static code analysis software which
>>>>> provides replacement cp and mv commands that track error reports in
>>>>> executables back to the source from which they are generated.
>>>>>
>>>>> jbsbug: https://bugs.openjdk.java.net/browse/JDK-8046765
>>>>> webrev: http://cr.openjdk.java.net/~mduigou/JDK-8046765/0/webrev/
>>>>>
>>>>> I've checked to make sure that patch doesn't change the build
>>>>> output on linux x64 and am currently checking on other platforms.
>>>>>
>>>>> It is probably easier to review this change by looking at the patch
>>>>> than by looking at the file diffs.
>>>>>
>>>>> Mike
>>>>>
>>>>
>>