KiCad / kicad-footprints

Official KiCad Footprint Libraries for Kicad version 5
https://kicad.github.io/footprints
Other
618 stars 713 forks source link

Travis check regression ? #1384

Open myfreescalewebpage opened 5 years ago

myfreescalewebpage commented 5 years ago

Since several weeks now the travis checks fails to verify footprint correclty...

Example on the addition of footprint SA15-11xxx (https://github.com/KiCad/kicad-footprints/pull/1246): https://travis-ci.org/KiCad/kicad-footprints/builds/489685609?utm_source=github_status&utm_medium=notification

poeschlr commented 5 years ago

Do you have a pull request with similar symptoms that is still open? I can not restart the build for that pull request as github no longer has the pointer to it (I would assume this is because it has been merged)

myfreescalewebpage commented 5 years ago

I have seen it several times in several PR but not found them anymore at the moment... Do you know if modifications have been made on the scripts recently ?

poeschlr commented 5 years ago

It did not seem like the problem is within the scripts. The scripts simply got a list of invalid files. (By properly getting invalid commits from travis, or more precisely: git diff with the commits from the environment variable $TRAVIS_COMMIT_RANGE listed non existing files for this pull request.)

Sadly i did not think about saving the original report so i can no longer really remember the exact wording but the part that triggered the output is this one: https://github.com/KiCad/kicad-library-utils/blob/88ba9a6c05a9414a352a602bddf67fb552d7d2b6/pcb/check_kicad_mod.py#L70-L72

diegoherranz commented 5 years ago

Example of a fail: https://travis-ci.org/KiCad/kicad-footprints/builds/492379737 From PR https://github.com/KiCad/kicad-footprints/pull/1066

That commit had the wrong 3D model path (it didn't match the footprint name) and still succeeded.

evanshultz commented 5 years ago

See https://github.com/KiCad/kicad-library-utils/blob/master/pcb/rules/F9_3.py. It looks like suffixes on the 3D model generate a warning, which allows one 3D model to be used for variations of an otherwise-identical footprint. If there are only warning Travis will still be green.

poeschlr commented 5 years ago

@diegoherranz yes your report seems at least to be similar but in this case diff is the program that complains not our script:

$ bash /home/travis/build/kicad-library-utils/pcb/travis/check_all.sh $TRAVIS_BUILD_DIR -v Commit range checked: 0c56f1a3cec18fb9eb38a65c92e8b4e638b15b14...b4046fca7e9d8b7accb32cbda8e8ccde790e98ae fatal: Invalid symmetric difference expression 0c56f1a3cec18fb9eb38a65c92e8b4e638b15b14...b4046fca7e9d8b7accb32cbda8e8ccde790e98ae

It seems travis is giving us invalid diff ranges for some pull requests. Edit: or we clone in a way that is incompatible with what we try to achieve.

Edit2: This is however different to how the originaly reported bug showed itself. There diff worked but gave non existing files as output meaning our script failed when it tried to read them.

Checking: MountingHole.pretty/DINRailAdapter_3xM3_PhoenixContact_1201578.kicad_mod File argument invalid: ['//home/travis/build/KiCad/kicad-footprints/MountingHole.pretty/DINRailAdapter_3xM3_PhoenixContact_1201578.kicad_mod']

Note that the PR in question did not touche mounting holes at all.

poeschlr commented 5 years ago

Found a pull request that i can use to analyze this: https://github.com/KiCad/kicad-footprints/pull/804

myfreescalewebpage commented 5 years ago

Yes @poeschlr that is exactly the same issue !

valerionew commented 5 years ago

I guess now we can add #1183 to the list too

valerionew commented 5 years ago

Hey guys! Please see: https://github.com/KiCad/kicad-footprints/pull/804#issuecomment-487363473

This appears to have fixed the issue two times

poeschlr commented 5 years ago

Ok i think i found what happens but have no clue why as i can not replicate it locally.

For travis a git diff c1...c2 seems to behave like starting git diff c1..c2 on my personal machine. (I use git 2.13 but travis is on 2.15 so it could be changed behavior, a bug or what ever. I also tested 2.7 which behaves the same as my 2.13.)