diff --git a/CHANGELOG.md b/CHANGELOG.md index 92cc6f47..d7c953c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `field_power` Name/s of the field/s used for the power raiting - `invalidate_pcb_text_cache` controls if we reset the text variables cached in the PCB file. + - `git_diff_strategy` selects how we preserve the current repo state. + (See #443) - Filters: - New `value_split` to extract information from the Value field and put it in separated fields. I.e. tolerance, voltage, etc. @@ -53,11 +55,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Command line: - `--list` also lists groups - KiCad v6/7 schematic: - - The hierarchy is expanded only if needed, i.e. value of an instance changed + - When saving an schematic the hierarchy is expanded only if needed, + i.e. value of an instance changed - List actions: - Now you must explicitly ask to configure outputs. Otherwise isn't needed. As a result you no longer need to have an SCH/PCB. Use `--config-outs` to get the old behavior. +- Git diff link file name: + - Now we default to using worktrees instead of stash push/pop. As a side + effect the names of the git points are chnaged. This is because main/master + only applies to the main worktree. So the names now refer to the closest + tag. ### Fixed - KiCad v6/7 schematic: diff --git a/README.md b/README.md index 5de98dd8..e9d0c05b 100644 --- a/README.md +++ b/README.md @@ -840,6 +840,10 @@ global: - `number`: [number=0] Error number we want to exclude. - `regex`: [string=''] Regular expression to match the text for the error we want to exclude. - *regexp*: Alias for regex. + - `git_diff_strategy`: [string='worktree'] [worktree,stash] When computing a PCB/SCH diff it configures how do we preserve the current + working state. The *worktree* mechanism creates a separated worktree, that then is just removed. + The *stash* mechanism uses *git stash push/pop* to save the current changes. Using *worktree* + is the preferred mechanism. - `hide_excluded`: [boolean=false] Default value for the `hide_excluded` option of various PCB outputs. - `impedance_controlled`: [boolean=false] The PCB needs specific dielectric characteristics. KiCad 6: you should set this in the Board Setup -> Physical Stackup. diff --git a/kibot/globals.py b/kibot/globals.py index 5e65bca8..7483d584 100644 --- a/kibot/globals.py +++ b/kibot/globals.py @@ -343,6 +343,11 @@ class Globals(FiltersOptions): variables update when using `set_text_variables`. You might want to disable it when applying some changes to the PCB and create a new copy to send to somebody without changing the cached values. The `auto` value will remove the cached values only when using `set_text_variables` """ + self.git_diff_strategy = 'worktree' + """ [worktree,stash] When computing a PCB/SCH diff it configures how do we preserve the current + working state. The *worktree* mechanism creates a separated worktree, that then is just removed. + The *stash* mechanism uses *git stash push/pop* to save the current changes. Using *worktree* + is the preferred mechanism """ self.set_doc('filters', " [list(dict)] KiBot warnings to be ignored ") self._filter_what = 'KiBot warnings' self.filters = FilterOptionsKiBot diff --git a/kibot/gs.py b/kibot/gs.py index 6c1b08ed..f24c515f 100644 --- a/kibot/gs.py +++ b/kibot/gs.py @@ -151,6 +151,7 @@ class GS(object): global_field_tolerance = None global_field_voltage = None global_hide_excluded = None + global_git_diff_strategy = None global_impedance_controlled = None global_invalidate_pcb_text_cache = None global_kiauto_time_out_scale = None diff --git a/kibot/out_diff.py b/kibot/out_diff.py index fa205d4d..3f8dd73b 100644 --- a/kibot/out_diff.py +++ b/kibot/out_diff.py @@ -154,51 +154,51 @@ class DiffOptions(BaseOptions): self.name_used_for_cache = name run_command(cmd) - def cache_pcb(self, name): + def cache_pcb(self, name, force_exist): if name: - if not os.path.isfile(name): + if not os.path.isfile(name) and not force_exist: raise KiPlotConfigurationError('Missing file to compare: `{}`'.format(name)) else: GS.check_pcb() name = GS.pcb_file - if not os.path.isfile(name): - if self.always_fail_if_missing: - raise KiPlotConfigurationError('Missing file to compare: `{}`'.format(name)) - with NamedTemporaryFile(mode='w', suffix='.kicad_pcb', delete=False) as f: - f.write("(kicad_pcb (version 20171130) (host pcbnew 5.1.5))\n") - name = f.name - self._to_remove.append(name) + if not os.path.isfile(name): + if self.always_fail_if_missing: + raise KiPlotConfigurationError('Missing file to compare: `{}`'.format(name)) + with NamedTemporaryFile(mode='w', suffix='.kicad_pcb', delete=False) as f: + f.write("(kicad_pcb (version 20171130) (host pcbnew 5.1.5))\n") + name = f.name + self._to_remove.append(name) hash = self.get_digest(name) self.add_to_cache(name, hash) return hash - def cache_sch(self, name): + def cache_sch(self, name, force_exist): if name: - if not os.path.isfile(name): + if not os.path.isfile(name) and not force_exist: raise KiPlotConfigurationError('Missing file to compare: `{}`'.format(name)) else: GS.check_sch() name = GS.sch_file + if not os.path.isfile(name): + if self.always_fail_if_missing: + raise KiPlotConfigurationError('Missing file to compare: `{}`'.format(name)) ext = os.path.splitext(name)[1] - if not os.path.isfile(name): - if self.always_fail_if_missing: - raise KiPlotConfigurationError('Missing file to compare: `{}`'.format(name)) - with NamedTemporaryFile(mode='w', suffix=ext, delete=False) as f: - logger.debug('Creating empty schematic: '+f.name) - if ext == '.kicad_sch': - f.write("(kicad_sch (version 20211123) (generator eeschema))\n") - else: - f.write("EESchema Schematic File Version 4\nEELAYER 30 0\nEELAYER END\n$Descr A4 11693 8268\n" - "$EndDescr\n$EndSCHEMATC\n") - name = f.name - self._to_remove.append(name) - if ext != '.kicad_sch': - lib_name = os.path.splitext(name)[0]+'-cache.lib' - if not os.path.isfile(lib_name): - logger.debug('Creating dummy cache lib: '+lib_name) - with open(lib_name, 'w') as f: - f.write("EESchema-LIBRARY Version 2.4\n#\n#End Library\n") - self._to_remove.append(lib_name) + with NamedTemporaryFile(mode='w', suffix=ext, delete=False) as f: + logger.debug('Creating empty schematic: '+f.name) + if ext == '.kicad_sch': + f.write("(kicad_sch (version 20211123) (generator eeschema))\n") + else: + f.write("EESchema Schematic File Version 4\nEELAYER 30 0\nEELAYER END\n$Descr A4 11693 8268\n" + "$EndDescr\n$EndSCHEMATC\n") + name = f.name + self._to_remove.append(name) + if ext != '.kicad_sch': + lib_name = os.path.splitext(name)[0]+'-cache.lib' + if not os.path.isfile(lib_name): + logger.debug('Creating dummy cache lib: '+lib_name) + with open(lib_name, 'w') as f: + f.write("EESchema-LIBRARY Version 2.4\n#\n#End Library\n") + self._to_remove.append(lib_name) # Schematics can have sub-sheets sch = load_any_sch(name, os.path.splitext(os.path.basename(name))[0]) files = sch.get_files() @@ -210,9 +210,9 @@ class DiffOptions(BaseOptions): self.add_to_cache(name, hash) return hash - def cache_file(self, name=None): + def cache_file(self, name=None, force_exist=False): self.git_hash = 'Current' if not name else 'FILE' - return self.cache_pcb(name) if self.pcb else self.cache_sch(name) + return self.cache_pcb(name, force_exist) if self.pcb else self.cache_sch(name, force_exist) def run_git(self, cmd, cwd=None, just_raise=False): if cwd is None: @@ -240,7 +240,7 @@ class DiffOptions(BaseOptions): logger.debug('Git submodules '+str(subs)) return subs - def undo_git(self): + def undo_git_use_stash(self): if self.checkedout: logger.debug('Restoring point '+self.branch) self.run_git(['checkout', '--force', '--recurse-submodules', self.branch]) @@ -303,23 +303,23 @@ class DiffOptions(BaseOptions): return self.solve_kibot_magic(name, 'KIBOT_TAG') return name - def get_git_point_desc(self, user_name): + def get_git_point_desc(self, user_name, cwd=None): # Are we at a tagged point? name = None try: - name = self.run_git(['describe', '--exact-match', '--tags', 'HEAD'], just_raise=True) + name = self.run_git(['describe', '--exact-match', '--tags', 'HEAD'], cwd=cwd, just_raise=True) if user_name == 'Dirty': name += '-dirty' except CalledProcessError: logger.debug("Not at a tag point") if name is None: # Are we at the HEAD of a branch? - branch = self.run_git(['rev-parse', '--abbrev-ref', 'HEAD']) + branch = self.run_git(['rev-parse', '--abbrev-ref', 'HEAD'], cwd=cwd) if branch == 'HEAD': # Detached state # Try to find the name relative to a tag try: - name = self.run_git(['describe', '--tags', '--dirty'], just_raise=True) + name = self.run_git(['describe', '--tags', '--dirty'], cwd=cwd, just_raise=True) except CalledProcessError: logger.debug("Can't find a tag name") if not name: @@ -331,19 +331,19 @@ class DiffOptions(BaseOptions): # Do we have a tag in this branch extra = None try: - extra = self.run_git(['describe', '--tags', '--abbrev=0', '--dirty'], just_raise=True) + extra = self.run_git(['describe', '--tags', '--abbrev=0', '--dirty'], cwd=cwd, just_raise=True) except CalledProcessError: logger.debug("Can't find a tag name") if extra: name += '['+extra+']' elif user_name == 'Dirty': name += '-dirty' - return '{}({})'.format(self.run_git(['rev-parse', '--short', 'HEAD']), name) + return '{}({})'.format(self.run_git(['rev-parse', '--short', 'HEAD'], cwd=cwd), name) def git_dirty(self): return self.run_git(['status', '--porcelain', '-uno']) - def cache_git(self, name): + def cache_git_use_stash(self, name): self.stashed = False self.checkedout = False # Which file @@ -385,9 +385,49 @@ class DiffOptions(BaseOptions): # A short version of the current hash self.git_hash = self.get_git_point_desc(name_ori) finally: - self.undo_git() + self.undo_git_use_stash() return hash + def cache_git(self, name): + self.stashed = False + self.checkedout = False + # Which file + if self.pcb: + GS.check_pcb() + self.file = GS.pcb_file + else: + GS.check_sch() + self.file = GS.sch_file + # Place where we know we have a repo + self.repo_dir = os.path.dirname(os.path.abspath(self.file)) + if name: + # Checkout the target + name_ori = name + name = self.solve_git_name(name) + git_tmp_wd = mkdtemp() + logger.debug('Checking out '+name+' to '+git_tmp_wd) + self.run_git(['worktree', 'add', git_tmp_wd, name]) + self._worktrees_to_remove.append(git_tmp_wd) + self.run_git(['submodule', 'init'], cwd=git_tmp_wd) + self.run_git(['submodule', 'update'], cwd=git_tmp_wd) + name_copy = self.run_git(['ls-files', '--full-name', self.file]) + name_copy = os.path.join(git_tmp_wd, name_copy) + logger.debug('- Using temporal copy: '+name_copy) + hash = self.cache_file(name_copy, force_exist=True) + cwd = git_tmp_wd + else: + name_ori = 'Dirty' if self.git_dirty() else 'HEAD' + # Populate the cache + hash = self.cache_file() + cwd = self.repo_dir + # A short version of the current hash + self.git_hash = self.get_git_point_desc(name_ori, cwd) + return hash + + def remove_git_worktree(self, name): + logger.debug('Removing temporal checkout at '+name) + self.run_git(['worktree', 'remove', '--force', name]) + @staticmethod def check_output_type(out, must_be): if out.type != must_be: @@ -422,16 +462,16 @@ class DiffOptions(BaseOptions): fname, dir_name = VariantOptions.save_tmp_dir_board('diff') else: dir_name = mkdtemp() - self.dirs_to_remove.append(dir_name) fname = GS.sch.save_variant(dir_name) GS.copy_project_sch(dir_name) + self.dirs_to_remove.append(dir_name) res = self.cache_file(os.path.join(dir_name, fname)) self.git_hash = 'Current' return res def cache_obj(self, name, type): if type == 'git': - return self.cache_git(name) + return self.cache_git(name) if GS.global_git_diff_strategy == 'worktree' else self.cache_git_use_stash(name) if type == 'file': return self.cache_file(name) if type == 'current': @@ -498,6 +538,7 @@ class DiffOptions(BaseOptions): def run(self, name): self.command = self.ensure_tool('KiDiff') self._to_remove = [] + self._worktrees_to_remove = [] if self.old_type == 'git' or self.new_type == 'git': self.git_command = self.ensure_tool('Git') if not self.pcb: @@ -543,6 +584,9 @@ class DiffOptions(BaseOptions): os.remove(self.incl_file) for f in self._to_remove: os.remove(f) + # Remove any git worktree that we created + for w in self._worktrees_to_remove: + self.remove_git_worktree(w) @output_class diff --git a/tests/test_plot/test_misc.py b/tests/test_plot/test_misc.py index 37ec7159..973d2623 100644 --- a/tests/test_plot/test_misc.py +++ b/tests/test_plot/test_misc.py @@ -1400,7 +1400,8 @@ def test_diff_git_2(test_dir): msg = f.read() assert msg == 'Bye!\n' # Check the link - assert glob(os.path.join(ctx.output_dir, prj+'-diff_pcb_*(v1)-*(master[[]v1[]]).pdf')) + assert (glob(os.path.join(ctx.output_dir, prj+'-diff_pcb_*(v1)-*(master[[]v1[]]).pdf')) + + glob(os.path.join(ctx.output_dir, prj+'-diff_pcb_*(v1)-*(v1-2-*).pdf'))), "Can't find link" ctx.clean_up(keep_project=True)