From be86ecf4a0699411ff18280987f7124c6375c00e Mon Sep 17 00:00:00 2001 From: Patrick Date: Mon, 4 Sep 2023 12:03:54 +0200 Subject: [PATCH 1/6] [GR-48321] Rebuild jars if excluded list changes --- mx_jardistribution.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/mx_jardistribution.py b/mx_jardistribution.py index 7578761a..cb82ae9c 100644 --- a/mx_jardistribution.py +++ b/mx_jardistribution.py @@ -337,6 +337,9 @@ def make_archive(self, javac_daemon=None): with mx.open(dependency_file, 'w') as fp: fp.write(jdk.home) + with mx.open(self._excluded_libs_save_file(), 'w') as fp: + fp.write(self._excluded_libs_as_json()) + if self.is_stripped(): self.strip_jar() @@ -591,6 +594,20 @@ def _module_info_as_json(self): module_infos = {name:getattr(self, name) for name in dir(self) if name == 'moduleInfo' or name.startswith('moduleInfo:')} return json.dumps(module_infos, sort_keys=True, indent=2) + def _excluded_libs_save_file(self) -> str: + """ + Gets the path to the file saving the list of excluded libraries + """ + return self.original_path() + ".excluded" + + def _excluded_libs_as_json(self) -> str: + """ + Gets the excludedLibs attribute as sorted json. + """ + import json + excluded_libs = {"excludedLibs": list(map(str, self.excludedLibs))} + return json.dumps(excluded_libs, sort_keys=True, indent=2) + def needsUpdate(self, newestInput): res = mx._needsUpdate(newestInput, self.path) if res: @@ -644,6 +661,17 @@ def needsUpdate(self, newestInput): pickle.load(fp) except ValueError as e: return f'Bad or incompatible module pickle: {e}' + + excluded_libs_file = self._excluded_libs_save_file() + if exists(excluded_libs_file): + excluded_libs = self._excluded_libs_as_json() + with mx.open(excluded_libs_file) as fp: + saved_excluded_libs = fp.read() + if excluded_libs != saved_excluded_libs: + import difflib + mx.log(f'{self} excluded libraries changed:' + os.linesep + ''.join(difflib.unified_diff(saved_excluded_libs.splitlines(1), excluded_libs.splitlines(1)))) + return 'excludedLibs changed' + if self.is_stripped(): previous_strip_configs = [] dependency_file = self.strip_config_dependency_file() From a1ca73858efc1b78803c3b743ad049fb966a398d Mon Sep 17 00:00:00 2001 From: Patrick Date: Mon, 4 Sep 2023 13:49:41 +0200 Subject: [PATCH 2/6] Merge moduleInfo and excludedLibs data files Both are stored in a single json file and used to determine if an update is necessary --- mx_jardistribution.py | 87 +++++++++++++++++++++---------------------- 1 file changed, 42 insertions(+), 45 deletions(-) diff --git a/mx_jardistribution.py b/mx_jardistribution.py index cb82ae9c..5d4143d9 100644 --- a/mx_jardistribution.py +++ b/mx_jardistribution.py @@ -210,7 +210,8 @@ def paths_to_clean(self): self.original_path() + _staging_dir_suffix, self.sourcesPath + _staging_dir_suffix, self._stripped_path(), - self.strip_mapping_file() + self.strip_mapping_file(), + self._config_save_file(), ] jdk = mx.get_jdk(tag='default') if jdk.javaCompliance >= '9': @@ -323,6 +324,9 @@ def make_archive(self, javac_daemon=None): latest_bin_archive = join(self.suite.get_output_root(False, False), "dists", os.path.basename(bin_archive.path)) _stage_file_impl(bin_archive.path, latest_bin_archive) + with mx.open(self._config_save_file(), 'w') as fp: + fp.write(self._config_as_json()) + self.notify_updated() compliance = self._compliance_for_build() if compliance is not None and compliance >= '9': @@ -330,16 +334,10 @@ def make_archive(self, javac_daemon=None): jmd = mx.make_java_module(self, jdk, stager.bin_archive, javac_daemon=javac_daemon) if jmd: setattr(self, '.javaModule', jmd) - mi_file = self._module_info_save_file() - with mx.open(mi_file, 'w') as fp: - fp.write(self._module_info_as_json()) dependency_file = self._jmod_build_jdk_dependency_file() with mx.open(dependency_file, 'w') as fp: fp.write(jdk.home) - with mx.open(self._excluded_libs_save_file(), 'w') as fp: - fp.write(self._excluded_libs_as_json()) - if self.is_stripped(): self.strip_jar() @@ -580,33 +578,40 @@ def _jmod_build_jdk_dependency_file(self): """ return self.original_path() + '.jdk' - def _module_info_save_file(self): + def _config_save_file(self) -> str: """ - Gets the path to the file saving `_module_info_as_json()`. + Gets the path to the file saving :meth:`_config_as_json`. """ - return self.original_path() + '.module-info' + return self.original_path() + ".json" - def _module_info_as_json(self): + def _config_as_json(self) -> str: """ - Gets the moduleInfo attribute(s) as sorted json. + Creates a sorted json dump of attributes that trigger a rebuild when they're changed (see :meth:`needsUpdate`). """ - import json - module_infos = {name:getattr(self, name) for name in dir(self) if name == 'moduleInfo' or name.startswith('moduleInfo:')} - return json.dumps(module_infos, sort_keys=True, indent=2) + config = {} - def _excluded_libs_save_file(self) -> str: - """ - Gets the path to the file saving the list of excluded libraries - """ - return self.original_path() + ".excluded" + def add_attribute(key, value): + """ + Adds an attribute to the config dict and makes sure no entry is overwritten. + + If entries are overwritten, the overwritten attribute is ignored when checking for updates. + """ + assert key not in config, f"Duplicate value in distribution config: {key}" + config[key] = value + + # The `exclude` list can change the jar file contents and needs to trigger a rebuild + add_attribute("excludedLibs", list(map(str, self.excludedLibs))) + + compliance = self._compliance_for_build() + if compliance is not None and compliance >= '9': + if mx.get_java_module_info(self): + # Module info change triggers a rebuild. + for name in dir(self): + if name == 'moduleInfo' or name.startswith('moduleInfo:'): + add_attribute(name, getattr(self, name)) - def _excluded_libs_as_json(self) -> str: - """ - Gets the excludedLibs attribute as sorted json. - """ import json - excluded_libs = {"excludedLibs": list(map(str, self.excludedLibs))} - return json.dumps(excluded_libs, sort_keys=True, indent=2) + return json.dumps(config, sort_keys=True, indent=2) def needsUpdate(self, newestInput): res = mx._needsUpdate(newestInput, self.path) @@ -638,17 +643,6 @@ def needsUpdate(self, newestInput): return res jdk = mx.get_jdk(compliance) - # Rebuild if module info changed - mi_file = self._module_info_save_file() - if exists(mi_file): - module_info = self._module_info_as_json() - with mx.open(mi_file) as fp: - saved_module_info = fp.read() - if module_info != saved_module_info: - import difflib - mx.log(f'{self} module info changed:' + os.linesep + ''.join(difflib.unified_diff(saved_module_info.splitlines(1), module_info.splitlines(1)))) - return 'module-info changed' - # Rebuild the jmod file if different JDK used previously dependency_file = self._jmod_build_jdk_dependency_file() if exists(dependency_file): @@ -662,15 +656,18 @@ def needsUpdate(self, newestInput): except ValueError as e: return f'Bad or incompatible module pickle: {e}' - excluded_libs_file = self._excluded_libs_save_file() - if exists(excluded_libs_file): - excluded_libs = self._excluded_libs_as_json() - with mx.open(excluded_libs_file) as fp: - saved_excluded_libs = fp.read() - if excluded_libs != saved_excluded_libs: + # Rebuild if the saved config file changed or doesn't exist + config_file = self._config_save_file() + if exists(config_file): + current_config = self._config_as_json() + with mx.open(config_file) as fp: + saved_config = fp.read() + if current_config != saved_config: import difflib - mx.log(f'{self} excluded libraries changed:' + os.linesep + ''.join(difflib.unified_diff(saved_excluded_libs.splitlines(1), excluded_libs.splitlines(1)))) - return 'excludedLibs changed' + mx.log(f'{self} distribution config changed:' + os.linesep + ''.join(difflib.unified_diff(saved_config.splitlines(True), current_config.splitlines(True)))) + return f'{config_file} changed' + else: + return f'{config_file} does not exist' if self.is_stripped(): previous_strip_configs = [] From 79be612a76c36dbf126c60ed0bffbb8e6e287f75 Mon Sep 17 00:00:00 2001 From: Patrick Date: Mon, 4 Sep 2023 14:36:35 +0200 Subject: [PATCH 3/6] Store jdk home in json dump instead of separate file --- mx_jardistribution.py | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/mx_jardistribution.py b/mx_jardistribution.py index 5d4143d9..cbc56f11 100644 --- a/mx_jardistribution.py +++ b/mx_jardistribution.py @@ -334,9 +334,6 @@ def make_archive(self, javac_daemon=None): jmd = mx.make_java_module(self, jdk, stager.bin_archive, javac_daemon=javac_daemon) if jmd: setattr(self, '.javaModule', jmd) - dependency_file = self._jmod_build_jdk_dependency_file() - with mx.open(dependency_file, 'w') as fp: - fp.write(jdk.home) if self.is_stripped(): self.strip_jar() @@ -571,13 +568,6 @@ def getArchivableResults(self, use_relpath=True, single=False): if self.is_stripped(): yield self.strip_mapping_file(), self.default_filename() + JARDistribution._strip_map_file_suffix - def _jmod_build_jdk_dependency_file(self): - """ - Gets the path to the file recording the JAVA_HOME of the JDK last used to - build the modular jar for this distribution. - """ - return self.original_path() + '.jdk' - def _config_save_file(self) -> str: """ Gets the path to the file saving :meth:`_config_as_json`. @@ -610,6 +600,10 @@ def add_attribute(key, value): if name == 'moduleInfo' or name.startswith('moduleInfo:'): add_attribute(name, getattr(self, name)) + # The jmod file needs to be rebuilt if a different JDK was used previously + jdk = mx.get_jdk(compliance) + add_attribute("jdk", str(jdk.home)) + import json return json.dumps(config, sort_keys=True, indent=2) @@ -641,15 +635,7 @@ def needsUpdate(self, newestInput): res = mx._needsUpdate(self.original_path(), pickle_path) if res: return res - jdk = mx.get_jdk(compliance) - # Rebuild the jmod file if different JDK used previously - dependency_file = self._jmod_build_jdk_dependency_file() - if exists(dependency_file): - with mx.open(dependency_file) as fp: - last_build_jdk = fp.read() - if last_build_jdk != jdk.home: - return f'build JDK changed from {last_build_jdk} to {jdk.home}' try: with open(pickle_path, 'rb') as fp: pickle.load(fp) From ee472a7f9a9891d33164f56281c80d1336507867 Mon Sep 17 00:00:00 2001 From: Patrick Date: Mon, 4 Sep 2023 15:12:47 +0200 Subject: [PATCH 4/6] Bump mx version --- mx.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mx.py b/mx.py index dc58272a..fe12b916 100755 --- a/mx.py +++ b/mx.py @@ -18783,7 +18783,7 @@ def alarm_handler(signum, frame): abort(1, killsig=signal.SIGINT) # The version must be updated for every PR (checked in CI) and the comment should reflect the PR's issue -version = VersionSpec("6.45.0") # multi-arch layout dirs +version = VersionSpec("6.45.1") # GR-48321 Fix JARDistribution exclude list not triggering rebuild _mx_start_datetime = datetime.utcnow() _last_timestamp = _mx_start_datetime From 85be17393ddb24232bb4b447351e5d55002d15b8 Mon Sep 17 00:00:00 2001 From: Patrick Date: Mon, 4 Sep 2023 15:13:29 +0200 Subject: [PATCH 5/6] Sync common.json --- common.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common.json b/common.json index 8b3f5ced..56e5ef00 100644 --- a/common.json +++ b/common.json @@ -4,7 +4,7 @@ "Jsonnet files should not include this file directly but use ci/common.jsonnet instead." ], - "mx_version": "6.44.2", + "mx_version": "6.45.0", "COMMENT.jdks": "When adding or removing JDKs keep in sync with JDKs in ci/common.jsonnet", "jdks": { From 970372d98db30e3f27374983e0bbecc424dfce0b Mon Sep 17 00:00:00 2001 From: Patrick Date: Mon, 4 Sep 2023 17:43:34 +0200 Subject: [PATCH 6/6] Improve comment --- mx_jardistribution.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mx_jardistribution.py b/mx_jardistribution.py index cbc56f11..0316338e 100644 --- a/mx_jardistribution.py +++ b/mx_jardistribution.py @@ -582,9 +582,9 @@ def _config_as_json(self) -> str: def add_attribute(key, value): """ - Adds an attribute to the config dict and makes sure no entry is overwritten. - - If entries are overwritten, the overwritten attribute is ignored when checking for updates. + Adds an attribute to the config dict and makes sure no entry is + overwritten, because otherwise attributes are ignored when checking + for updated. """ assert key not in config, f"Duplicate value in distribution config: {key}" config[key] = value