Re: [Patch V2] Build spec: add description for build with binary cache

[Laszlo Ersek <lersek@redhat.com>]

On 09/30/17 04:49, Gao, Liming wrote:
> Laszlo:
> Yes. Delete .hash can invalidate hash way. But, --hash option is to
> try hash first. If hash has no change, there is no AutoGen and Make.
> If hash is change, AutoGen and Make will still be trigged. So, for
> your case, if any file is changed,  it will cause hash value be
> changed, then trig time-stamp Make build. I don't think you need to
> invalidate hash.

I've done the following test:

number hash         subject                                                              commit timestamp
------ ------------ -------------------------------------------------------------------- --------------------------
     1 8932679df5be MdeModulePkg/DxeCore: Add check to ensure no possible NULL ptr deref 2017-09-26 09:38:46 +08:00
     2 1b8eca8b1aff BaseTools: report build time measured by module of EDKII Build       2017-09-26 13:39:39 +08:00
     3 36d083ef0018 BaseTools: add support for BIOS build with binary cache              2017-09-27 17:14:54 +08:00
     4 119d8c42ab8b BaseTools: Fix the regression bug to build single module             2017-09-28 10:10:39 +08:00
     5 62634215f320 ShellPkg/UefiHandleParsingLib.c: Map SmmPciRootBridgeIo correctly    2017-09-29 01:08:07 +08:00
     6 770f3f6144b4 ShellPkg/dh: Add the 'dh' dump support for Partition Info protocol   2017-09-29 09:39:35 +08:00

I built the tree at commits #6, #1 and #5. The idea is that commit #3
introduced "--hash", so it separates #1 from #5 and #6.

Commits #2 and #4 are also used for separation, because #2 introduced a
build regression, and #4 fixed it. This means that all commits before
commit #2 lack "--hash" *and* do not have the regression, and all
commits after #4 have "--hash" and also do not have the regression.

Additionally, both commits #5 and #6 modify the file
"ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c", so
moving from #6, to #1, to #5 (as if we were bisecting) requires
recompiling "UefiHandleParsingLib.c" at every step.

(1) First build: at commit #6 (770f3f6144b4). This build was done from
zero.

The build uses "--hash". "UefiHandleParsingLib.c" is compiled.

(2) Second build: at commit #1 (8932679df5be).

"--hash" is not available, so it is not used. "UefiHandleParsingLib.c"
is compiled correctly. (I checked in the build log.)

(3) Third build: at commit #5 (62634215f320). "UefiHandleParsingLib.c"
is compiled correctly. (I checked in the build log.)

So it indeed looks like I don't need to invalidate the hash manually.

Thanks!
Laszlo


>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Thursday, September 28, 2017 8:07 PM
>> To: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Shaw, Kevin W
>> <kevin.w.shaw@intel.com>; Gao, Liming <liming.gao@intel.com>
>> Subject: Re: [edk2] [Patch V2] Build spec: add description for build with binary
>> cache
>>
>> On 09/19/17 08:48, Yonghong Zhu wrote:
>>> V2:
>>> change the option name to --binary-destination and --binary-source.
>>>
>>> fixes:https://bugzilla.tianocore.org/show_bug.cgi?id=689
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Cc: Michael Kinney <michael.d.kinney@intel.com>
>>> Cc: Kevin W Shaw <kevin.w.shaw@intel.com>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
>>> ---
>>>  .../82_auto-generation_process.md                    | 20
>> ++++++++++++++++++++
>>>  README.md                                            |  1 +
>>>  appendix_d_buildexe_command/d4_usage.md              | 19
>> +++++++++++++++----
>>>  3 files changed, 36 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/8_pre-build_autogen_stage/82_auto-generation_process.md
>> b/8_pre-build_autogen_stage/82_auto-generation_process.md
>>> index 671a7d5..f2ddf32 100644
>>> --- a/8_pre-build_autogen_stage/82_auto-generation_process.md
>>> +++ b/8_pre-build_autogen_stage/82_auto-generation_process.md
>>> @@ -1031,10 +1031,30 @@ maximum command line length.  The default
>> value is 4096.
>>>  **Note:** The following `FLAGS` options are included in the response file:
>>>  `PP_FLAGS`, `CC_FLAGS`, `VFRPP_FLAGS`, `APP_FLAGS`, `ASLPP_FLAGS`,
>> `ASLCC_FLAGS`,
>>>  and `ASM_FLAGS`.
>>>  **********
>>>
>>> +#### 8.2.4.15 Build with Binary Cache
>>> +
>>> +**build** tool provides three new options for binary cache feature.
>>> +--hash enables hash-based caching during build process. when --hash is
>> enabled,
>>> +build tool will base on the module hash value to do the incremental build,
>> without
>>> +--hash, build tool will base on the timestamp to do the incremental build. --
>> hash
>>> +option use md5 method to get every hash value, DSC/FDF, tools_def.txt,
>> build_rule.txt
>>> +and build command are calculated as global hash value, Package DEC and its
>> include
>>> +header files are calculated as package hash value, Module source files and
>> its INF
>>> +file are calculated as module hash value. Library hash value will combine
>> the global
>>> +hash value and its dependent package hash value. Driver hash value will
>> combine the
>>> +global hash value, its dependent package hash value and its linked library
>> hash value.
>>> +When --hash and --binary-destination are specified, build tool will copy the
>> generated
>>> +binary files for each module into the directory specified by binary-
>> destination at the
>>> +build phase. Binary-destination directory caches all the generated binary
>> files.
>>> +When --hash and --binary-source are specified, build tool will try to get the
>> binary
>>> +files from the binary source directory at the build phase. If the cached
>> binary has
>>> +the same hash value, it will be directly used. Otherwise, build tool will
>> compile the
>>> +source files and generate the binary files.
>>> +
>>
>> I have another question about this feature: how can I invalidate the
>> binary cache, so that the next invocation with "--hash" fall back to the
>> timestamp-based incremental build?
>>
>> Is it enough if I remove all "*.hash" files from the Build tree?
>>
>> This question is relevant for bisection. Assume that we are bisecting a
>> commit range that straddles the introduction of "--hash" to BaseTools,
>> and that we use a build script that uses "--hash" whenever it is available.
>>
>> Consider the following build order:
>>
>> (1) Build the tree with "--hash" (because "--hash" is available at the
>> commit that we're testing). Assume this build displays the symptom we
>> are bisecting, so we enter "git bisect bad", and git checks out an
>> earlier commit.
>>
>> (2) Assume BaseTools lacks "--hash" at this earlier commit, so we build
>> the tree without "--hash". Using the timestamp checks, the modules
>> changed or affected by the checkout done by git-bisect in step (1) will
>> be rebuilt correctly. We test this build and find that it works okay, so
>> we enter "git bisect good". Git checks out a later commit (such that it
>> is earlier than the one tested in (1)).
>>
>> (3) Assume that at this commit, "--hash" is again available, so we build
>> the tree with "--hash". This is incorrect however, because the cached
>> values come from step (1), not step (2).
>>
>>
>> This means that every time the build script notices that "--hash" is
>> unavailable, it must invalidate (delete) all the cached values, so that
>> *next time* it returns to using "--hash", all the hash values are
>> recalculated from scratch, and only the timestamp based checks are used
>> for that build.
>>
>> So, how can I invalidate all the cached values? Is it enough to delete
>> the *.hash files?
>>
>> Thanks,
>> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>