On 2020-04-30 10:21, Jan Lahoda wrote:
On 30. 04. 20 9:49, Magnus Ihse Bursie wrote:
On 2020-04-30 09:03, Jan Lahoda wrote:
Hi,
The building of lib/ct.sym is not reproducible, due to timestamps of
files inside the file (which is basically a zip file).
The proposed solution is to use a constant timestamp for the files
inside the ct.sym file. To simplify the construction, the
CreateSymbols tool does not produce files on the filesystem anymore,
but rather constructs the ct.sym directly.
Thanks for helping us address reproducibility issues!
If this truly makes ct.sym reproducible, you can include this patch
as well to make the compare script test this:
diff --git a/make/scripts/compare.sh b/make/scripts/compare.sh
--- a/make/scripts/compare.sh
+++ b/make/scripts/compare.sh
@@ -394,7 +394,7 @@
GENERAL_FILES=$(cd $THIS_DIR && $FIND . -type f ! -name "*.so"
! -name "*.jar" \
! -name "*.zip" ! -name "*.debuginfo" ! -name "*.dylib" !
-name "jexec" \
- ! -name "modules" ! -name "ct.sym" ! -name "*.diz" ! -name
"*.dll" \
+ ! -name "modules" ! -name "*.diz" ! -name "*.dll" \
! -name "*.cpl" ! -name "*.pdb" ! -name "*.exp" ! -name
"*.ilk" \
! -name "*.lib" ! -name "*.war" ! -name "*.jmod" ! -name
"*.exe" \
! -name "*.obj" ! -name "*.o" ! -name "jspawnhelper" !
-name "*.a" \
Sure, will do.
Proposed webrev: http://cr.openjdk.java.net/~jlahoda/8241616/webrev.00/
The build aspect of the changes look good.
I have a question that is not directly related to the changes you are
doing in this fix. Why do we need to call two consecutive tools? (And
I notice you changed the calling order in this patch). Do the first
tool write a temporary file (where, if so?), and the second one makes
the final ct.sym? Or does the first tool create ct.sym and the second
one updates it?
It is not completely necessary to have two tools, just
TransitiveDependencies take variable number of arguments; which is
less convenient to do into "CreateSymbols build-ctsym", but I think it
can be done. Let me look into that. (But it is definitely the first
variant - TransitiveDependencies builds system-modules file, which
CreateSymbols then incorporates into ct.sym).
Okay, if that's the case then everything is fine. You need to do no
changes in the build tool. You can leave things as they are right now,
and I can split it up in two SetupExecute calls instead later on.
I can't see how the output from TransitiveDependencies is specified.
Maybe it's hard-coded? If anything, I'd appreciate that the input and
output files from build tools are specified explicitly on the
command-line. That makes is so much easier to follow what happens.
/Magnus
Jan
If it's the former case, it would be better to split the make rule
into two: one where the target of the rule is the temporary file fir
the first command, and one where the real ct.sym is the target, and
the temporary file is a dependency.
If it's the second case, I'd really like to have this fixed. Build
tools that modify stuff in place is the worst. If the build e.g.
breaks down in the second command, an incremental re-run will think
the target is done, and not retry this command. Since both commands
are classes in the same package, maybe they could be merged into a
single call?
If you believe a change like this would be out of scope for this bug,
I understand.
/Magnus
JBS: https://bugs.openjdk.java.net/browse/JDK-8241616
How does this look?
Thanks,
Jan
