KiDiff

[mdeweerd]

@set-soft I started testing KiDiff. I think that the "Issues" are related to the KiBot setup, I am not creating an issue (as of yet), but writing down observations as I go along.

Submodules

I use submodules to refer to my libraries which appears in .git/config as follows:

[submodule "LIBS/kicad-ynamics-lib"]
        url = ssh://git@SERVER:PORT/NAME/PROJECT.git
        active = true
        update = merge

It appears that submodules are not checked out/update with regards to the project tag. Not an issue for me, but should at least be mentioned as a warning in the documentation.

[mdeweerd]

Got the fllowing in my verbose log:

Switched to branch 'master'
Your branch is up to date with 'origin/master'.
 (kibot - out_diff.py:37)
DEBUG:Restoring changes (kibot - out_diff.py:147)
DEBUG:Executing: ['/usr/bin/git', 'stash', 'pop'] (kibot - out_diff.py:41)
ERROR:Running ['/usr/bin/git', 'stash', 'pop'] returned 1 (kibot - out_diff.py:45)
DEBUG:- Output from command: No stash entries found.

and without verbose:

ERROR:Running ['/usr/bin/git', 'stash', 'pop'] returned 1 (kibot - out_diff.py:45)

Does not happen when a file is changed (and not committed).

[set-soft]

Got the fllowing in my verbose log:

Thanks! I didn't know about this asymmetry: push returns 0 if nothing to push, pop returns 1 if nothing to pop ...
Now should work.

[mdeweerd]

I got this when diffing the schematic:

ERROR:Running ['/usr/bin/kicad-diff.py', '-vvvv', '--only_cache', '--old_file_hash', 'schd096a20d3bdbc089d735159ef0ff0e0713780496', '--cache_dir', '.cache', '/root/workdir/STAR-LCD-DUO.sch', '/root/workdir/STAR-LCD-DUO.sch'] returned 3 (kibot - out_diff.py:45)

Apparently, eeschema does not understand --output_name .

DEBUG:kicad-diff.py:Executing: ['eeschema_do', 'export', '--file_format', 'pdf', '--monochrome', '--no_frame', '--output_name', '.cache/schd096a20d3bdbc089d735159ef0ff0e0713780496/Schematic.pdf', '/root/workdir/STAR-LCD-DUO.sch', '.']
usage: eeschema_do [-h] [--record] [--rec_width REC_WIDTH]
                   [--rec_height REC_HEIGHT] [--start_x11vnc] [--use_wm]
                   [--verbose] [--version] [--wait_key]
                   [--wait_start WAIT_START] [--time_out_scale TIME_OUT_SCALE]
                   {export,run_erc,netlist,bom_xml} ... schematic output_dir
eeschema_do: error: unrecognized arguments: --output_name .cache/schd096a20d3bdbc089d735159ef0ff0e0713780496/Schematic.pdf
ERROR:kicad-diff.py:Failed to plot .cache/schd096a20d3bdbc089d735159ef0ff0e0713780496/Schematic.pdf
 (kibot - out_diff.py:37)

I am using the dev image up-to-date:

dev: Pulling from setsoft/kicad_auto
Digest: sha256:9934cc7f342c9fda9326661f85e3235c49eb6de3b0a7e510d22d623537fe7fa5
Status: Image is up to date for setsoft/kicad_auto:dev

EDIT: Also tried setsoft/kicad_auto_test:latest but does not have 'diff'.

[mdeweerd]

In addition to the above: how about an option to kibot to log kibot-check to a file in the output directory so that it is automatically log which tools were (last) used to generate the outputs. It would only be correct if all files were generated "at the same time" or without updates to the tools, but I tend to clean/move the output directory when making a release.

[set-soft]

version = '1.5.15

This is an ancient version, not the one inside the docker image. Did you run the grep command from inside the docker image? If not: you have 1.5.15 installed in your WSL setup, and you are getting it because this is most probably requested from your home (which I guess you are mounting when using the docker image)

maybe add "KiAuto" on that line?

I added the logic to show it for cases like KiAuto and KiDiff.

[mdeweerd]

Possibly in the cases where kibot_check reports that some tools are too old, let kibot warn/stop (according to some option).

[mdeweerd]

Ok that was completely my fault - I tried it on an old project and the script running kibot in docker still had the line that overrided kiauto with the local version...
Sorry for that.

However, that shows that a kibot-check integrated in kibot that could warn/log/block is useful ;-).

[set-soft]

KiBot should try to download a fresh copy if you have a very old installed. If KiBot can't get a copy with the correct version should abort. The problem with diff is that KiAuto is a dependency of the dependency, but I'm adding it as a direct dependency anyways.

[mdeweerd]

So I got a result for comparing two PCB git versions, the SCH diff did not work (see above).
Good work already!

Regarding the naming of the outputfile: it can of course be hardcoded in the filename, but in order to avoid repetition, it would be nice to be able to reference the git references in the name. Either the short hashes (independeng of how the version was specified), or the tag name (specified tag name or tag present on the version when the pcb/schematic is unmodified vs. git).

I can make separate "Issues" for all of the above or combine them except for the feature request (reference hash or tag in filename for differences).

[mdeweerd]

I tested this - there is a problem with the link itself, and another issue as well which is requires to at least change the linkage.

1. The link: in my case the link is useable from Cygwin, but not from the File Explorer (windows). Maybe that is because I start docker from Cygwin but I would be surprised that this impact the symbolic links as other windows programs dont like them.

2. After multiple runs there would be multiple links pointing to the same file which changes, while the link name itself has the version information. So I prefer the copy, or a fixed link from the generic name to the more specific name. In case of the latter (link from the generic name to the specific name), make's requirements on the timestamp should still be ok as the link itself is updated as well as the target of the link.

EDIT: Windows has a link feature since a very long time - not only "modern windows", but Cygwin and Windows diverge on how that is implemented.

[set-soft]

I don't fully understand the case: are you using docker AND WSL? One of them should be enough. And in both cases Cygwin seems to be redundant.
According to Microsoft you can just use WSL and run Linux commands from the command prompt, so you don't need Cygwin. They also claim you can start Windows applications even from a bash shell on WSL.
When using WSL you shouldn't need the docker image, unless you need to run 2 different KiCad versions, and even in this case you can have more than one Linux on WSL.
Microsoft recommends copying the files to the WSL side before processing (performance issues when crossing file systems). Are you sure that a symlink created by a Linux running on the WSL environment, created on /mnt/c/ isn't translated to a valid Windows link?

BTW: When I say modern Windows I refer to the Windows NT branch, not the old Win32 (i.e. Windows 95, 98, Millenium, etc.)

[mdeweerd]

I am used to cygwin and cygwin is more transparent than WSL is - there is no need for running a container or VM either. I tried WSL a few times, it's not very practical to use.
I will look into it again at a later time, I do it more or less once a year. It's not only about what you can do, but also how (easy) you can do it.

As far as I understand it, Docker uses the WSL framework. Using Docker has its advantages.

Copying - I am not convinced that that is the issue, especially when cloning from inside the container. I am close to having a workflow that can do everything in a single run in the container.

BTW! Modern windows: as I've been working with Windows NT "branch" since 1995 a timespan of more than 25 years (and I started using windows with Windows 3.1, after ZX81, ZX Spectrum, and concurrently with Sinclair QL), Windows NT (branch) is no longer a standard for "Modern".

I installed 'strace' and examined what happens with the .pro file.

112   stat("/root/workdir/STAR-LCD-DUO.pro", {st_mode=S_IFREG|0644, st_size=3934, ...}) = 0
112   stat("/root/workdir/STAR-LCD-DUO.pro", {st_mode=S_IFREG|0644, st_size=3934, ...}) = 0
112   rename("/root/workdir/STAR-LCD-DUO.pro", "/root/workdir/STAR-LCD-DUO.pro-bak") = 0
112   openat(AT_FDCWD, "/root/workdir/STAR-LCD-DUO.pro", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 5

Its not clear what process 112 is, but it is the process launching several comand, and one of them is:

112   execve("/usr/bin/eeschema_do", ["eeschema_do", "export", "--file_format", "pdf", "--monochrome", "--no_frame", "--output_name", ".cache/schd096a20d3bdbc089d73515"..., "/root/workdir/STAR-LCD-DUO.sch", "."], 0x7ffd8bbad5a0 /* 20 vars */ <unfinished ...>

It looks like it's not putting the original file back.

[set-soft]

BTW! Modern windows: as I've been working with Windows NT "branch" since 1995 a timespan of more than 25 years (and I started using windows with Windows 3.1, after ZX81, ZX Spectrum, and concurrently with Sinclair QL), Windows NT (branch) is no longer a standard for "Modern".

According to Microsoft Windows NT was a change in the design of the Windows kernel, introducing the HAL and moving the Win32 stuff to an API. This is what modern Windows is. It also introduced NTFS, the filesystem that supports links, unlike VFAT that fakes links.
I also used Windows 3.1, and programmed for it, and this is when I realized it wasn't usable as an OS.
My first computer was a ZX81, just with a different name "Timex Sinclair 1000", this was the name used here. Then I moved to a Commodore 128. And finally to the PCs (staring with an XT using a NEC V20 CPU), I even developed ISA boards for this computer, doing DMA at 1 MB/s almost blocked the bus :-))))

It looks like it's not putting the original file back.

The file is loaded to memory and then restored. I never though using text mode open could generate any problem ...

[mdeweerd]

Yeah, text mode generates more issues than you'ld expect - fixed a test case in some project one that that wasn't really testing what it should because it was writing and reading post-processed data (due to the text mode).

Still, it don't call a system modern because it's based on WinNT :-) - with windows 3.1

[set-soft]

It appears that submodules are not checked out/update with regards to the project tag. Not an issue for me, but should at least be mentioned as a warning in the documentation.

KiBot is doing git checkout xxxx, I think that this should be enough. Any idea?

[set-soft]

Done, v2.0.3 should fix it. Docker images should be automatically generated in a few minutes.

[mdeweerd]

More than a few minutes, but it arrived and works when cloning everything in the container.

Still issues when using the clone made outside docker - I may find the real reason at some point.
It is currently failing because a file is missing in the submodules on recursive checkout...

I do not have changes in my submodules, but to be complete they would have to be stashed as well... (git submodule foreach 'git stash' )

[mdeweerd]

According to my tests, git checkout -f --recurse-submodules REFERENCE is the solution to the blocking checkout. However, the submodules end up detached. So this requires to check (and store) the HEAD of each submodule before any action.

git submodule foreach git stash -m NAMEworks, but to pop them, one seems to need to enter ech submodule `( cd "submoduledir" ; git checkout REFERENCE)` after checkinggit submodule foreach git stash list'

[set-soft]

a4abb65 should add this.
Note that recursive submodules aren't supported.

The -f will go as an option.

[set-soft]

e34d4a0 adds the force_checkout option to add --force

[mdeweerd]

Given that the git issue and my local kiauto issue are now fixed, I could procede with the flow to generated difference files for the PCB and the SCH, but the SCH generated the PCB difference.

Further I am questioning myself how the difference filename is going to reflect PCB or SCH through my global naming scheme.

Here are the essential parts of my yaml configuration, I can share more if needed - I was using the proto variant, but I do not think that is essential:

import:
   # Some imports
global:
  output: '%f_%r_%d-%i.%x'
  # out_dir: "%f_%br_%bd_%g/"
  out_dir: '%bf_%r_%d_%g/'
  date_time_format: '%Y%m%d_%H%M%S'
  date_format: '%Y%m%d'
  time_format: '%H%M%S'
  variant: 5V_3btn
variants:
  - name: proto
    comment: 5V power, 4 buttons per screen
    type: kicost
    file_id: _proto
    variant: ^(button3|button4|screen5v|conn3|AD8|bl_buffer)$
    pre_transform: kicost_rename
  - name: diff_A_pcb
    type: diff
    comment: PCB difference with previous version
    layers: [all]
    options:
      # output: '%f-%i%I%v.%x'
      cache_dir: .cache
      diff_mode: red_green # or stats
      # fuzz: 5
      #new: AYAC100577-02_20191209
      #new_type: git  # git or file
      old: DUO_PROTO
      old_type: git
      pcb: true
      threshold: 40
  - name: diff_A_sch
    type: diff
    comment: SCH difference with previous version
    layers: [all]
    options:
      # output: '%f-%i%I%v.%x'
      cache_dir: .cache
      diff_mode: red_green # or stats
      # fuzz: 5
      #new: AYAC100577-02_20191209
      #new_type: git  # git or file
      old: DUO_PROTO
      old_type: git
      pcb: false
      threshold: 40

First I generated both diff_A_pcb and diff_A_sch, but noticing that I got the PCB differences, I then deleted the difference PDF that was generated and only requested to produce diff_A_sch - I still got the PCB difference.
The log suggests that the Schematic difference is generated.

[set-soft]

Given that the git issue and my local kiauto issue are now fixed, I could procede with the flow to generated difference files for the PCB and the SCH, but the SCH generated the PCB difference.

You are right, they must be identified differently. Now %i is diff_pcb for PCBs and diff_sch for Schematics

[mdeweerd]

Side effects of stash without local changes, and old stash on stack.
Kibot should only stash pop when it did really stash something.

^[[36m^[[1mDEBUG:Executing: ['/usr/bin/git', 'stash', 'push', '-m', 'KiBot_Changes_Entry'] (kibot - out_diff.py:46)^[[0m
^[[36m^[[1mDEBUG:- Output from command: No local changes to save

^[[36m^[[1mDEBUG:Executing: ['/usr/bin/git', 'stash', 'pop', '--index'] (kibot - out_diff.py:46)^[[0m
^[[31m^[[1mERROR:Running ['/usr/bin/git', 'stash', 'pop', '--index'] returned 1 (kibot - out_diff.py:50)^[[0m
^[[36m^[[1mDEBUG:- Output from command: Auto-merging runInKiAuto
CONFLICT (content): Merge conflict in runInKiAuto

[set-soft]

This is already solved. KiBot only pops when the top of the stack has a magic message

[mdeweerd]

I also suggest to add '--' in the calls to git as the tags might start with the - sign and be interpreted as an option (not my case, but could happen).

[set-soft]

Doesn't seem to be possible, things like: /usr/bin/git checkout --recurse-submodules -- HEAD doesn't work

[mdeweerd]

My offer to create issues instead still stands - it maybe more efficient at this point.

[set-soft]

Do it, but only for things we will solve.
Various things you mention are already solved or must be solved outside KiBot.

[mdeweerd]

Actually it all seems to be fixed in KiBot now (at least it works with the project git clone I have been using).
And this is "new" which happens when creating the difference twice:

FileExistsError: [Errno 17] File exists: 'STAR-LCD-DUO_01_20211217-_sch.pdf' -> '/root/workdir/STAR-LCD-DUO_01_20211217__proto/STAR-LCD-DUO_01_20211217-_sch_e769c13-None.pdf'

[mdeweerd]

And other issue: still getting pcb differences for the schematic.

Git is giving trouble again, also when the clone was managed in docker. Strange.

When starting from a clean clone using docker:

ERROR:Running ['/usr/bin/git', 'checkout', '--recurse-submodules', 'master'] returned 1 (kibot - out_diff.py:50)
DEBUG:- Output from command: error: Your local changes to the following files would be overwritten by checkout:
        STAR-LCD-DUO.pro

[set-soft]

Again: Double check you don't have a mess with the upper/lower case names.
Try copying the repo to the Linux filesystem, not mounting the NTFS

[mdeweerd]

In case of failure one out of two times I do not get into the original git state in my clone. So I starting investigating mounting an overlay over the docker workdir and write the output elsewhere.

[set-soft]

KiBot tries to restore the old state, even after crashing (2 layers of finally statements are implemented). But if you get a fail during the restore process (double fault) things won't be restored.
We need to find which situations can generate such a problem and find ways to workaround them.