kicad-developers team mailing list archive

[José Ignacio <jose.cyborg@xxxxxxxxx>]

The user would have to click through a big fat warning that the file is
from the future. If you wanna be doubly sure, open things in read only mode
if there is a version mismatch, so the user at worst can save the file with
another name, allowing recovery without having to drop into a text editor.


On Tue, Mar 20, 2018 at 9:48 AM, Wayne Stambaugh <stambaughw@xxxxxxxxx>
wrote:

> Jeff,
>
> I wanted some time to think about this.  It has been discussed before.
> While I'm not completely opposed to it, I still haven't found a more
> compelling argument to convince me that it is a better idea than using
> strict parsing.  As a user, it does have a certain appeal.  As a
> developer, it opens a Pandora's box of issues that once they are in
> play, could be extremely difficult to reverse.  Please see my responses
> below.
>
> On 3/20/2018 9:40 AM, Jeff Young wrote:
> > @Wayne, did you have any thoughts on this iteration?
> >
> >> On 20 Mar 2018, at 10:22, Jeff Young <jeff@xxxxxxxxx
> >> <mailto:jeff@xxxxxxxxx>> wrote:
> >>
> >> Oh, and I don’t think this materially alters the support equation: we
> >> already have to deal with hand-edited boards, so what’s in the file is
> >> never guaranteed to be something Kicad put there.
>
> True, but this is one of the reasons that the board parser is strict.
> It is immediately obvious what doesn't belong in the file.  Also, in the
> past users have tricked eeschema and pcbnew into a feature that didn't
> technically exist by taking advantage of the loose file parsing.  Then
> when something changed internally and their clever hacks were broken, we
> ended up with bug reports.  I do not want to repeat this again.
>
> >>
> >>> On 20 Mar 2018, at 10:19, Jeff Young <jeff@xxxxxxxxx
> >>> <mailto:jeff@xxxxxxxxx>> wrote:
> >>>
> >>> Hi Seth,
> >>>
> >>> The original version spit out log entries for skipped elements.  This
> >>> version follows the XML/browser convention of silently ignoring.
> >>>  Even though this isn’t an XML format, I think we need to recognise
> >>> that we live in an XML world and XML processing conventions set
> >>> expectations.
>
> After reading this, I am even more thankful that we didn't choose XML
> for our file format.  Gleefully ignoring file syntax that the parser
> doesn't understand IMO is a bad idea.  This flies in the face of the
> basic premise that we control the format of our files not someone else.
> As soon as you allow others to dictate your file format, you are in
> trouble.
>
> >>>
> >>> The patch strictly checks everything for round-tripping so that there
> >>> is no data loss.  The pad stuff is really a separate issue: it was
> >>> meant to be loose only during development and then tightened up, but
> >>> the tightening step was forgotten.  Since we don’t store pad stuff we
> >>> don’t understand, it has to be tightened.  In short: if you can round
> >>> trip stuff you don’t understand then do so; otherwise throw.
>
> The round tripping is fine and makes sense but it also adds to the code
> complexity of the parser.  The pad issue is different.  I don't know
> when that was slipped into the parser but it should not be there.  If
> there is an unexpected token, that should flag a file load error.
>
> >>>
> >>> Certainly one use case is opening boards from future versions.  If
> >>> you edit them, then you’re at your own peril.  This behaviour is
> >>> common enough that I believe it is well understood (although we
> >>> should obviously mention it in a version-check dialog).
>
> Perhaps, but I just see this ending badly.  Maybe I'm being paranoid
> here but it's so easy to imagine a scenario where someone did a lot of
> editing in a newer version of kicad then makes the fatal mistake of
> opening the board in an older version of kicad and wipes out a lot of
> work.  Technically not kicad's fault but I'm not so sure the user will
> see it that way.
>
> >>>
> >>> Another use case is 3rd-party tools (which might even be written as
> >>> Python plugins) that want to carry their own stuff around in the
> >>> board.  These might even be processing/manufacturing instructions
> >>> that don’t have any visual expression in Kicad anyway.
>
> This is one of the primary reasons that I do not like ignoring unknown
> file formatting.  It creates the potential for name space pollution that
> could cause issues down the road.  The eventual goal is to implement the
> "property" token to define key/value pairs for third party applications
> to add user specific information to any board object without polluting
> the controlled part of the file format.  The beauty of this is that we
> do not have to coordinate with 3rd party developers to ensure we are not
> clashing with anything the are working on.  They are free to add
> whatever properties they would like while we still maintain strict file
> parsing.  This was always part of the grand plan for the board file
> format that kind of got lost in the noise and me becoming project leader.
>
> Cheers,
>
> Wayne
>
> >>>
> >>> Cheers,
> >>> Jeff.
> >>>
> >>>
> >>>> On 19 Mar 2018, at 22:51, Seth Hillbrand <seth.hillbrand@xxxxxxxxx
> >>>> <mailto:seth.hillbrand@xxxxxxxxx>> wrote:
> >>>>
> >>>> Hi Jeff-
> >>>>
> >>>> A few questions on how you are implementing this:
> >>>>
> >>>> 1) How does the user know what was skipped?  I can imagine team
> >>>> members with different versions getting into difficulty, especially
> >>>> if the features being skipped change the board layout.
> >>>>
> >>>> 2) You are removing strict checking for most of the board but you
> >>>> are adding strict checking for pad options. What's the difference?
> >>>>
> >>>> 3) If we keep these options around but don't allow editing/removing,
> >>>> don't we run into a risk of generating a really wonk-y board that
> >>>> only looks wonky in a newer version of KiCad but looks fine in an
> >>>> older version?  For example, imagine we implement rounded corners as
> >>>> a parameter and then a user with an older version of KiCad edits and
> >>>> saves the board.  The rounded corner is only visible in KiCad 6 but
> >>>> the user in KiCad 5 can edit the board to look right to her only to
> >>>> have her colleague use KiCad 6 and see the track violating DRC.  I
> >>>> think that might be counter-intuitive for users but maybe there's a
> >>>> way around it.
> >>>>
> >>>> In general, if I understand the use-case, this is to allow users to
> >>>> open newer boards in older KiCad versions?  Is there another use case?
> >>>>
> >>>> I think I can see clear to this for options like the 3d-model offset
> >>>> where it could be either inches or mm but there isn't a difference
> >>>> in the actual layout.  Allowing general unrecognized options would
> >>>> seem to open up a worm can in terms of support as in "Please post
> >>>> the KiCad version and the file version in order to figure out the
> >>>> problem."
> >>>>
> >>>> -S
> >>>>
> >>>>
> >>>>
> >>>> 2018-03-18 9:46 GMT-07:00 Jeff Young <jeff@xxxxxxxxx
> >>>> <mailto:jeff@xxxxxxxxx>>:
> >>>>
> >>>>     OK, for your guys’ (re)viewing pleasure, a patch which
> >>>>     losslessly round-trips stuff it doesn’t understand.
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>     On 16 Mar 2018, at 19:15, hauptmech <hauptmech@xxxxxxxxx
> >>>>>     <mailto:hauptmech@xxxxxxxxx>> wrote:
> >>>>>
> >>>>>     While i would still like to see this (my) shim go in, I agree
> >>>>>     with wayne. Until/unless a less brittle approach is used for
> >>>>>     the parser, it's better to signal a problem painfully and
> >>>>>     maintain the integrity of the file.
> >>>>>
> >>>>>     I have to admit that when i first heard the s-expressions idea
> >>>>>     I assumed that the intention was to use a lisp like virtual
> >>>>>     machine for loading and maintaining design data. I've used vm's
> >>>>>     for data storage before (lua and python), and it's great.
> >>>>>     Parsing is free and you can jam in functions and other data
> >>>>>     when needed.
> >>>>>
> >>>>>
> >>>>>
> >>>>>     On 17 Mar 2018 07:17, "Jeff Young" <jeff@xxxxxxxxx
> >>>>>     <mailto:jeff@xxxxxxxxx>> wrote:
> >>>>>
> >>>>>         Hi Wayne,
> >>>>>
> >>>>>         Patch reverted.
> >>>>>
> >>>>>         However, I think your concern is misplaced.  If we want to
> >>>>>         prevent dataloss (ie: from reading a 6.0 file into 5.0),
> >>>>>         then we should warn the user based on the version string
> >>>>>         (and give them a chance to say “I still want to open”).
> >>>>>
> >>>>>         But either way, actually failing to read the file leaves
> >>>>>         the user in a pickle (especially when it’s easy enough for
> >>>>>         them to try out a nightly that may very well be ahead of
> >>>>>         their stable).
> >>>>>
> >>>>>         (And for that reason I think it’s important to put it into
> >>>>>         5.0, as otherwise it won’t help until we start making 7.0
> >>>>>         file format changes.)
> >>>>>
> >>>>>         Cheers,
> >>>>>         Jeff.
> >>>>>
> >>>>>         > On 16 Mar 2018, at 18:00, Wayne Stambaugh
> >>>>>         <stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>> wrote:
> >>>>>         >
> >>>>>         > Jeff,
> >>>>>         >
> >>>>>         > Please revert this patch.  Any changes to the board file
> >>>>>         parser and/or
> >>>>>         > formatter need to be discussed before the changes are
> >>>>>         committed.  It has
> >>>>>         > been the intention that the board parser be pendantic by
> >>>>>         design to
> >>>>>         > prevent data loss by ignoring unknown formatting.
> >>>>>         Allowing anything
> >>>>>         > outside of the expected formatting in the board file is
> >>>>>         not something
> >>>>>         > that I want to introduce without some discussion.  Even
> >>>>>         should we decide
> >>>>>         > to accept this patch, I would prefer we put it off until
> v6.
> >>>>>         >
> >>>>>         > That being said, the patch fails to build on windows with
> >>>>>         following error:
> >>>>>         >
> >>>>>         >
> >>>>>         C:/msys64/home/wstambaugh/src/kicad-trunk/pcbnew/pcb_parser.
> cpp:
> >>>>>         In
> >>>>>         > member function 'void PCB_PARSER::parseUnknown()':
> >>>>>         >
> >>>>>         C:/msys64/home/wstambaugh/src/kicad-trunk/pcbnew/pcb_parser.
> cpp:1269:12:
> >>>>>         > error: request for member 'LogText' in
> >>>>>         '__acrt_iob_func(2)', which is of
> >>>>>         > pointer type  FILE* {aka _iobuf*}' (maybe you meant to
> >>>>>         use '->' ?)
> >>>>>         >     stderr.LogText( msg );
> >>>>>         >            ^~~~~~~
> >>>>>         >
> >>>>>         > Cheers,
> >>>>>         >
> >>>>>         > Wayne
> >>>>>         >
> >>>>>         > On 3/16/2018 1:08 PM, Jeff Young wrote:
> >>>>>         >> Perhaps somewhat germane to this discussion I have
> >>>>>         removed the strict-format nags from the PCB parser.  It now
> >>>>>         logs warnings to stderr but otherwise ignores structures it
> >>>>>         doesn’t understand.
> >>>>>         >>
> >>>>>         >> I’m not sure that helps hauptmech much as if the file is
> >>>>>         subsequently written the unknown markup will be lost, but I
> >>>>>         thought I’d mention it.
> >>>>>         >>
> >>>>>         >> Cheers,
> >>>>>         >> Jeff.
> >>>>>         >>
> >>>>>         >>> On 7 Mar 2018, at 20:12, hauptmech <hauptmech@xxxxxxxxx
> >>>>>         <mailto:hauptmech@xxxxxxxxx>> wrote:
> >>>>>         >>>
> >>>>>         >>> Hi Thomasz,
> >>>>>         >>>
> >>>>>         >>> I hope I'm able to address you concerns. I'm stuck
> >>>>>         using kicad stable in many situations. I brought clearances
> >>>>>         up for discussion last July but no one moved on it, nor are
> >>>>>         they required to. Clearance management is a major pain
> >>>>>         point for me so I wrote a fix. This patch will let us (me
> >>>>>         and the people I collaborate with) work using version 5,
> >>>>>         but open and close files written with a version patched
> >>>>>         with clearance handling code.
> >>>>>         >>>
> >>>>>         >>> I believe that code exactly like this will go into
> >>>>>         version 6. Getting it in earlier makes a huge difference to
> >>>>>         me, so I'm submitting it.
> >>>>>         >>>
> >>>>>         >>> On 07/03/18 23:30, Tomasz Wlostowski wrote:
> >>>>>         >>>> Hi hauptmech,
> >>>>>         >>>>
> >>>>>         >>>> I'm sorry but IMHO we can't accept your patch:
> >>>>>         >>>> - it changes the file format while we are already in
> >>>>>         feature freeze.
> >>>>>         >>>> This is a way too big change to accept for the V5.
> >>>>>         >>>
> >>>>>         >>> This patch simply adds tokens to the file format. No
> >>>>>         clearance data is saved for files that use the netclass
> >>>>>         only. Files without clearance tokens continue to remain
> >>>>>         without them.
> >>>>>         >>>
> >>>>>         >>> Yes it is a backward compatible file format change, but
> >>>>>         it does no harm to V5 files already in the wild.
> >>>>>         >>>
> >>>>>         >>>> - we are planning to overhaul the clearance/design
> >>>>>         rules system in V6.
> >>>>>         >>>> Storing the clearance *DIRECTLY* for each track
> >>>>>         segment/via will
> >>>>>         >>>> conflict with any more sophisticated design rule
> >>>>>         management system.
> >>>>>         >>>>
> >>>>>         >>> I'm glad you are planning this. I am sure that
> >>>>>         regardless of the sophistication of the rule system, you
> >>>>>         will store clearance directly for exactly the same reason
> >>>>>         that track width is stored directly now. There are always
> >>>>>         exceptions to the rules.
> >>>>>         >>>
> >>>>>         >>> If kicad choose a direction that does not store
> >>>>>         clearances per item, then it is easy to rip these few lines
> >>>>>         back out.
> >>>>>         >>>
> >>>>>         >>> Did this answer your existing concerns about this
> >>>>>         patch? Are there any other concerns you have about this
> patch?
> >>>>>         >>>
> >>>>>         >>>
> >>>>>         >>>
> >>>>>         >>> _______________________________________________
> >>>>>         >>> Mailing list: https://launchpad.net/~kicad-developers
> >>>>>         <https://launchpad.net/~kicad-developers>
> >>>>>         >>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >>>>>         <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> >>>>>         >>> Unsubscribe : https://launchpad.net/~kicad-developers
> >>>>>         <https://launchpad.net/~kicad-developers>
> >>>>>         >>> More help   : https://help.launchpad.net/ListHelp
> >>>>>         <https://help.launchpad.net/ListHelp>
> >>>>>         >>
> >>>>>         >>
> >>>>>         >> _______________________________________________
> >>>>>         >> Mailing list: https://launchpad.net/~kicad-developers
> >>>>>         <https://launchpad.net/~kicad-developers>
> >>>>>         >> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >>>>>         <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> >>>>>         >> Unsubscribe : https://launchpad.net/~kicad-developers
> >>>>>         <https://launchpad.net/~kicad-developers>
> >>>>>         >> More help   : https://help.launchpad.net/ListHelp
> >>>>>         <https://help.launchpad.net/ListHelp>
> >>>>>         >>
> >>>>>         >
> >>>>>         > _______________________________________________
> >>>>>         > Mailing list: https://launchpad.net/~kicad-developers
> >>>>>         <https://launchpad.net/~kicad-developers>
> >>>>>         > Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >>>>>         <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> >>>>>         > Unsubscribe : https://launchpad.net/~kicad-developers
> >>>>>         <https://launchpad.net/~kicad-developers>
> >>>>>         > More help   : https://help.launchpad.net/ListHelp
> >>>>>         <https://help.launchpad.net/ListHelp>
> >>>>>
> >>>>>
> >>>>>         _______________________________________________
> >>>>>         Mailing list: https://launchpad.net/~kicad-developers
> >>>>>         <https://launchpad.net/~kicad-developers>
> >>>>>         Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >>>>>         <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> >>>>>         Unsubscribe : https://launchpad.net/~kicad-developers
> >>>>>         <https://launchpad.net/~kicad-developers>
> >>>>>         More help   : https://help.launchpad.net/ListHelp
> >>>>>         <https://help.launchpad.net/ListHelp>
> >>>>>
> >>>>
> >>>>
> >>>>     _______________________________________________
> >>>>     Mailing list: https://launchpad.net/~kicad-developers
> >>>>     <https://launchpad.net/~kicad-developers>
> >>>>     Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >>>>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> >>>>     Unsubscribe : https://launchpad.net/~kicad-developers
> >>>>     <https://launchpad.net/~kicad-developers>
> >>>>     More help   : https://help.launchpad.net/ListHelp
> >>>>     <https://help.launchpad.net/ListHelp>
> >>>>
> >>>>
> >>>
> >>> _______________________________________________
> >>> Mailing list: https://launchpad.net/~kicad-developers
> >>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >>> <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> >>> Unsubscribe : https://launchpad.net/~kicad-developers
> >>> More help   : https://help.launchpad.net/ListHelp
> >>
> >> _______________________________________________
> >> Mailing list: https://launchpad.net/~kicad-developers
> >> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >> <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> >> Unsubscribe : https://launchpad.net/~kicad-developers
> >> More help   : https://help.launchpad.net/ListHelp
> >
> >
> >
> > _______________________________________________
> > Mailing list: https://launchpad.net/~kicad-developers
> > Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> > Unsubscribe : https://launchpad.net/~kicad-developers
> > More help   : https://help.launchpad.net/ListHelp
> >
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>