Added top-level comaprator script #4008

judovana · 2024-10-25T10:22:46Z

Hello! I believe the #3991 was closed due to not understanding waht it is doing.

This script is not doing any rebuild, and thus is not comparing on identity, but on binary identically builds only. As it is, it do ot have any similar work present in the toolchain. Can you please reconsider its inclusion?

maybe the in file/main readme:

# This is top level script for comparing two jdks.  It 
# This script expects exactly two args - tag:os.arch/dir/archive, two times
# eg: compare_builds.sh  /usr/lib/jvm/java-21-openjdk /my/home/my_custom_tarball.tar.xz
# eg: compare_builds.sh  /my/custom/dir/with_jdk jdk-21.0.4+7:windows.x64
# if the parameter is tag, the temurin jdk of given tag:os.arch is downloaded
# yo can omit the :os.arch part, then it will be guessed... but you may know better
# It copies and creates all in CWD!!!!
# Pass the files/dirs as absolute values, it should not be that hard
# It is useful to have JAVA_HOME pointing to some other JDK used to compile and run supporting files
# otherwise one of the provided JDKs will be duplicated, and used
# for your own safety, do not run it as root
#
# The JDK you are comparing is launched to get various info about itself
# If you need to compare jdks on different platform then they are for, you need to go manually.

is misleading?

judovana · 2024-10-31T10:27:51Z

Thje label is not correct this is cygwin (thus windows) and linux

…hted "only in" files

${} instead of $ $(()) instead of let quoting basename instead of dirname

…g results

mostly disabled SC2002

judovana · 2024-11-05T08:45:23Z

Hello good people. Anybody around? The #3991 was closed as duplicate, and there is no duplicate toolchain. All the tools are written, but the readme is quite complex, and one must follow it. In addition there are quite a few corenr cases.

This script remains valid contribution, which is putting it all together. I found small intersection with detection of signtool, but thats all.

karianna · 2024-11-05T22:12:05Z

needs linter fixes to start with.

judovana · 2024-11-06T11:48:15Z

@karianna the linter is all green, but the job never finishes and timeouts:(

karianna · 2024-11-10T02:06:16Z

I see:

File:[/github/workspace/tooling/compare-builds.sh]
2024-11-10 01:46:57 [WARN] Warnings found in [bash-exec] linter!
2024-11-10 01:46:57 [WARN] Error: File:[/github/workspace/tooling/compare-builds.sh] is not executable

judovana · 2024-11-11T09:08:08Z

thanx.I had missed it.Fixed

…

-- Mgr. Jiri Vanek ***@***.***

---------- Původní e-mail ---------- Od: Martijn Verburg ***@***.***> Komu: adoptium/temurin-build ***@***.***> Kopie: judovana ***@***.***>, Author ***@***.***> Datum: 10. 11. 2024 3:06:44 Předmět: Re: [adoptium/temurin-build] Added top-level comaprator script (PR #4008) " I see: File:[/github/workspace/tooling/compare-builds.sh] 2024-11-10 01:46:57 [WARN] Warnings found in [bash-exec] linter! 2024-11-10 01:46:57 [WARN] Error: File:[/github/workspace/tooling/compare- builds.sh] is not executable — Reply to this email directly, view it on GitHub (#4008 (comment)) , or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAWFCSZB5U7GIOXAB7YSWHTZ725S3AVCNFSM6AAAAABQS6ZDBGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRWGU2DQNRRGI) . You are receiving this because you authored the thread. Message ID: < ***@***.***> "

karianna · 2024-11-11T21:37:41Z

OK so the linter fails on JSCPD, ew have a config file for this, have you tried running locally?

judovana · 2024-11-12T15:53:42Z

Even the local npx jscpd -v compare-builds.sh runs for ever. The method of tapsAndJunits is guilty. Up to local totalOnlyIn is ok, but then more and more lines is added, it runs longer^lines so since some 5 lines I had never seen an end. Similarly when cutting from bottom, from aprox printXmlTest "compare" "compared-files-count" "2" "${totalFile}" "../artifact/$(basename "${totalFile}")" >> "${unitFile}" it starts to go longer^lines again.

the

 if [ "${totalDiffs}" -eq 0 ] ; then
      passed=$(("${passed}"+1))
    else
      failed=$(("${failed}"+1))
    fi
    if [ "${totalFiles}" -ne 0 ] ; then
      passed=$(("${passed}"+1))
    else
      failed=$(("${failed}"+1))
    fi
    if [ "${totalOnlyIn}" -eq 0 ] ; then
      passed=$(("${passed}"+1))
    else
      failed=$(("${failed}"+1))
    fi
    total=$(("${passed}"+"${failed}"))
    printXmlHeader "${passed}" "${failed}" "${total}" 0 "compare-comparable-builds" > "${unitFile}"
    tapHeader "${total}"  "$(date)" > "${resultsTapFile}"
    if [ "${totalDiffs}" -eq 0 ] ; then
      printXmlTest "compare" "differences-count" "1" "" "../artifact/$(basename "${differencesFile}")" >> "${unitFile}"
      tapTestStart "ok" "1" "differences-count" >> "${resultsTapFile}"
      echo "-- no differences --"
    else
      printXmlTest "compare" "differences-count" "1" "${differencesFile}" "../artifact/$(basename "${differencesFile}")" >> "${unitFile}"
      tapTestStart "not ok" "1" "differences-count" >> "${resultsTapFile}"
      echo "-- differences count: $totalDiffs --"
    fi
    set +e
      tapFromWholeFile "${differencesFile}" "${differencesFile}" >> "${resultsTapFile}"
      tapTestEnd >> "${resultsTapFile}"
    set -e
    if [ "${totalFiles}" -ne 0 ] ; then
      printXmlTest "compare" "compared-files-count" "2" "" "../artifact/$(basename "${totalFile}")" >> "${unitFile}"
      tapTestStart "ok" "2" "compared-files-count" >> "${resultsTapFile}"
      echo "-- files compared: $totalFiles --"
    else
      printXmlTest "compare" "compared-files-count" "2" "${totalFile}" "../artifact/$(basename "${totalFile}")" >> "${unitFile}"
      tapTestStart "not ok" "2" "compared-files-count" >> "${resultsTapFile}"
      echo "-- no files compared --"
    fi
    set +e
      tapFromWholeFile "${totalFile}" "${totalFile}" >> "${resultsTapFile}"
      tapTestEnd >> "${resultsTapFile}"
    set -e
    if [ "${totalOnlyIn}" -eq 0 ] ; then
      printXmlTest "compare" "onlyin-count" "3" "" "../artifact/$(basename "${differencesFile}")" >> "${unitFile}"
      tapTestStart "ok" "3" "onlyin-count" >> "${resultsTapFile}"
      echo "-- no onlyin files --"
    else
      printXmlTest "compare" "onlyin-count" "3" "${differencesFile}" "../artifact/$(basename "${differencesFile}")" >> "${unitFile}"
      tapTestStart "not ok" "3" "onlyin-count" >> "${resultsTapFile}"
      echo "-- onlyin files count: $totalOnlyIn --"
    fi
    set +e
      tapFromWholeFile "${differencesFile}" "${differencesFile}" >> "${resultsTapFile}"
      tapTestEnd >> "${resultsTapFile}"
    set -e
    if [ $GLOABL_RESULT -eq 0 ] ; then
      printXmlTest "compare" "comparable-builds" "4" "" "../artifact/$(basename "${diffFileParam}")" >> "${unitFile}"
      tapTestStart "ok" "4" "comparable-builds" >> "${resultsTapFile}"
      echo "-- COMPARABLE --"
    else
      printXmlTest "compare" "comparable-builds" "4" "${diffFileParam}" "../artifact/$(basename "${diffFileParam}")" >> "${unitFile}"
      tapTestStart "not ok" "4" "comparable-builds" >> "${resultsTapFile}"
      echo "-- MISMATCH --"
    fi

Is malicious. That code is obviously so nasty it deadlocks the tool. I will decompose it a bit.

judovana · 2024-11-12T16:02:16Z

On contrary, when rest of the file is removed, and only "that" code is left in, it is in few ms finsihed.

judovana · 2024-11-14T15:03:49Z

No success to narrow it more. Filed kucherenko/jscpd#716

Will redactor the tapsAndJunits method to be more human (and thus JSCPD) friendly

sxa · 2024-12-04T12:37:38Z

Since there is a lot of code here I haven't gone through everything that it has done yet, but can you clarify what the intent of this is and how it is different from the comparison tools we do after attempting a binary rebuild?

For example we have the Compare_JDK function at

temurin-build/tooling/reproducible/windows_repro_build_compare.sh

Line 576 in 8646220

Compare_JDK() {

which is executed for comparing a new rebuild with an existing build (Usually one from the Temurin CI). We definitely don't want to have two separate implementations of "compare two builds and report on the differences" but if this is genuinely doing something completely different let us know.

judovana · 2024-12-04T14:06:04Z

Hi! thanx for reply. I definitely agree that there must be no duplicated code. If any "top level comparator" would just call set, or even only one, function(s) from various shared libraries, it would be ok. I was last checking the state of code in shared libraries, in earl October and all I needed for such code was missing. Maybe it already got better?

Primarily, it should be calling the tools from Andrew(which requires some set-up and detection, which it must do too), which are checking not just identical state, but binary comparability - https://github.com/adoptium/temurin-build/blob/master/tooling/reproducible/ReproducibleBuilds.md#comparable-build-tools - optionally, why not. Sometimes you nedd them, sometimes not. As far as I can read it, those are still not called.
Secondarily it have to take care of various corner-cases - like doing the work on copy if you are comparing directories, as the jdk after the check is destroyed. With this hand in hand, it can take good care of third jdk which you need (to run https://github.com/adoptium/temurin-build/tree/master/tooling/src/java/temurin/tools), with unpacking of the tarabll
Then many small tweeks, like eg downloading the reference temurin, which I want to compare my build against.
Minor thing - there should be one script for windows, linux and mac

Maybe there are similar functions already on place, but I do not see them. Quick check:

no usage of BinRepl.java
no usage of WindowsUpdateVsVersionInfo.c
no usage of comparable_patch.sh
no help with downloading reference temurin
not unified handling of 3rd jdk
no detection of -XshowSettings 2>&1| grep 'java.vendor (and similar) for extraction from binaries

I found detection of signtool.exe and cl.exe and msvs cBuild tools.

I believe that standalone, reusable script, should be proudly presented, not hidden, and if it exists, it should be called from other work-horses, not the opposite - because the comparison of binary-comparable is standalone and tricky task, and it is super useful for much wider audience.

sxa · 2024-12-04T15:26:39Z

because the comparison of binary-comparable is standalone and tricky task, and it is super useful for much wider audience.

Yeah I agree on that. We do have articles about running the stuff we have listed on https://adoptium.net/en-GB/blog/2024/08/adoptium-reproducible-verification-builds/ (and my now a little out of date as things have moved on videos such as this one which talks about the automation we have in place.

On the specifics you mention:

repro_common.sh uses BinRepl
comparable_patch.sh is mentioned in https://github.com/adoptium/temurin-build/blob/master/tooling/reproducible/ReproducibleBuilds.md for people who want to use it although @andrew-m-leonard can probably comment on whether it's used by anything we specifically run at the project. I suspect not if it's only used for comparing Temurin with other vendors.
no detection of -XshowSettings 2>&1| grep 'java.vendor (and similar) for extraction from binaries is likely correct since if the binaries are considered the same that may not be too important.

judovana · 2024-12-05T09:59:59Z

because the comparison of binary-comparable is standalone and tricky task, and it is super useful for much wider audience.

Yeah I agree on that. We do have articles about running the stuff we have listed on https://adoptium.net/en-GB/blog/2024/08/adoptium-reproducible-verification-builds/ (and my now a little out of date as things have moved on videos such as this one which talks about the automation we have in place.

I meant more via reusable script then via talk/readme:) But this is nice one.

On the specifics you mention:

* [repro_common.sh](https://github.com/adoptium/temurin-build/blob/master/tooling/reproducible/repro_common.sh) uses BinRepl

Thanx! I had missed that one. So the script I'm proposing probably can lost some part of logic by calling several functions from various repro_comon (and maybe others, as some are in windows only script and so on). It is quite hard to read.

* comparable_patch.sh is mentioned in https://github.com/adoptium/temurin-build/blob/master/tooling/reproducible/ReproducibleBuilds.md for people who want to use it although @andrew-m-leonard can probably comment on whether it's used by anything we specifically run at the project. I suspect not if it's only used for comparing Temurin with other vendors.

* `no detection of  -XshowSettings 2>&1| grep 'java.vendor (and similar) for extraction from binaries` is likely correct since if the binaries are considered the same that may not be too important.

Yup, thas it, The identical logic is moreover indeed around, but the binary-comaprable logic is not.

judovana · 2024-12-10T09:54:30Z

@sxa I just realised I did not asked one clarification: " two separate implementations ". I'm still failing to see the first one.

I'm generally reading it, that there are some function, which have duplication inside this script. So if I remove the existing duplicated "bodies" (in this script) and replace them by existing functions, such a script should become acceptable?

judovana added 4 commits October 24, 2024 17:48

Added top-level comaprator script

c4d2528

Added license

e88e5a1

Quoting where possible and other shelllint issues

04df973

One more litner issue

074aeb2

github-actions bot added the windows Issues that affect or relate to the WINDOWS OS label Oct 25, 2024

This was referenced Oct 31, 2024

repro_common should do all the comparison work in _CP directory #4013

Open

add documentation for top level reusable comparator script #3998

Open

judovana added 11 commits November 1, 2024 16:22

Restricted results to check at least something was diffed and highlig…

910c9a9

…hted "only in" files

bashlint

647ea78

${} instead of $ $(()) instead of let quoting basename instead of dirname

Made greps returning reasonably on fail

6cde5a6

makeing seds no fatal, so the comparable build can produce all passin…

c09edb3

…g results

tap files honouring number of tests

83e9568

Making the whole section with only in check non fatal

02dd02d

bashlint

dbbf382

mostly disabled SC2002

roeprly closing tap parts

90f02f2

fixing SC2002

2c6b212

SC2129 Consider using { cmd1; cmd2; } >> file

aafc389

repllaced grep | wc by grep -c

59c8d02

changed permissions to 755 and renamed bat.bat

d9b712d

made compare-builds.sh executable

907db52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added top-level comaprator script #4008

Added top-level comaprator script #4008

judovana commented Oct 25, 2024

judovana commented Oct 31, 2024 •

edited

Loading

judovana commented Nov 5, 2024

karianna commented Nov 5, 2024

judovana commented Nov 6, 2024

karianna commented Nov 10, 2024

judovana commented Nov 11, 2024 via email

karianna commented Nov 11, 2024

judovana commented Nov 12, 2024 •

edited

Loading

judovana commented Nov 12, 2024

judovana commented Nov 14, 2024

sxa commented Dec 4, 2024

judovana commented Dec 4, 2024

sxa commented Dec 4, 2024

judovana commented Dec 5, 2024

judovana commented Dec 10, 2024

Added top-level comaprator script #4008

Are you sure you want to change the base?

Added top-level comaprator script #4008

Conversation

judovana commented Oct 25, 2024

judovana commented Oct 31, 2024 • edited Loading

judovana commented Nov 5, 2024

karianna commented Nov 5, 2024

judovana commented Nov 6, 2024

karianna commented Nov 10, 2024

judovana commented Nov 11, 2024 via email

karianna commented Nov 11, 2024

judovana commented Nov 12, 2024 • edited Loading

judovana commented Nov 12, 2024

judovana commented Nov 14, 2024

sxa commented Dec 4, 2024

judovana commented Dec 4, 2024

sxa commented Dec 4, 2024

judovana commented Dec 5, 2024

judovana commented Dec 10, 2024

judovana commented Oct 31, 2024 •

edited

Loading

judovana commented Nov 12, 2024 •

edited

Loading