LibreCAD / libdxfrw

Successor of https://sourceforge.net/projects/libdxfrw/, developed for LibreCAD, by LibreCAD Contributors, usable for all
GNU General Public License v2.0
193 stars 72 forks source link

Recommended upstream? #22

Open nyalldawson opened 3 years ago

nyalldawson commented 3 years ago

I'd like to forward some patches from qgis external copy of libdxfrw (eg https://github.com/qgis/QGIS/pull/44428), but I'd like clarification on whether this repo or https://github.com/codelibs/libdxfrw would be the best place to do this.

Ideally these three maintained forks could be unified into one upstream. Is there general interest for this?

lordofbikes commented 3 years ago

This repo is the successor of https://sourceforge.net/projects/libdxfrw/, which is the origin of libdxfrw. There were some discussion and efforts about various forks recently, but I have no conclusion yet. I'm aware of the codelibs fork, but I'm not sure about their changes. From my last check, couple months ago, I think that this repo is the most recent and still platform independent. There are some other forks, which diverged to run on single OS only.

What I can remember, this is the first attempt to push fork changes back to this upstream. This is more than welcome, many thanks. I'll try to check your changes soon and see how our repos diverged.

nyalldawson commented 3 years ago

I'll try to check your changes soon and see how our repos diverged.

I'm more than happy to do this and submit PRs, if you are indeed seeking for this to be a software-agnostic upstream and not a librecad specific fork of the library... :+1:

ghost commented 3 years ago

JFTR, Here is libdxfrw fork used in SolveSpace:

phkahler commented 2 years ago

@lordofbikes I'd like to delete the solvespace fork and just use this repo if possible since the reason (lack of CMAKE) for forking has apparently been resolved. Could you create an actual release at some point (maybe along with the next librecad release) so we can point to a specific version rather than an arbitrary commit?

EDIT: There are also a number (38 commits) of fixes in our repo. Maybe we should bring some of them over here. LibreCAD isn't really merging PRs very fast, so would this be worth my time?

lordofbikes commented 2 years ago

@phkahler this fork was initially created for LibreCAD_3 only, which uses CMAKE too. LibreCAD 2 has libdxfrw still bundled yet, but is in sync with current HEAD.

Latetely we decided to make this repo the official successor of the Sourceforge repo, because there are no active contributors for years now. I have permission to the Sourceforge repo, but we want to leave it untouched yet as we have no response from Rallaz, the founder. A link to GitHub is already placed there on the summary page.

There are a bunch of improvements made to libdxfrw, spread over a couple forks, which we already consolidated here and we want to continue this work. Then we need of course a release near-term.

For a release there is still some work to do. Having the repo for LibreCAD only, made changes to the interface easy, without breaking too much. But as an official source we must consider others needs too and be more careful to avoid deal braking changes. This is true for fork maintainers too, we must consider together then, which changes must be implemented in libdxfrw and which are probably too specific and should be kept in the project interface implementation when possible. I'm open to any suggestions and support.

E.g. the mentioned CVE's, fixes are included in current HEAD only, but the current HEAD has an interface modification which requires to instantiate 2 abstract methods by the implementer. See description of #20 for details. Means, we must consider how to handle this now and in future. We can add default implementations to libdxfrw or back port the CVE fixes. But iirc, these two methods are part of bug fixes in LibreCAD 2 and therefor I think back porting doesn't make much sense.

There are already contributions from QGIS fork, with more to come and the same attempt to use this repo for QGIS in future. So you're welcome to participate here too.

Concerning the merge pace you mentioned, this is simply due to the fact that I'm the only active maintainer for LibreCAD currently. I'm near to finally release the long overdue LibreCAD 2.2.0 release, then I can focus back to a libdxfrw release.

rpavlik commented 2 years ago

FWIW, I did rebase the Solvespace changes here: https://github.com/codelibs/libdxfrw/pull/19 and made some additional cleanups here: https://github.com/codelibs/libdxfrw/pull/20

TFreudi1 commented 2 years ago

@rpavlik: You changed some constructors, for example DRW_LWPolyline(const DRW_LWPolyline &) = delete; DRW_LWPolyline(DRW_LWPolyline &&) = default; but with that the project dwg2dxf won't compile.

See code on the end ... arround row 70 in dx_iface.h that won't compile and .. shame on me ... but I don't know how to change to your new constructor. I just refreshed my knowledge about references but maybe I'm to old :-)

virtual void addLWPolyline(const DRW_LWPolyline& data){ currentBlock->ent.push_back(new DRW_LWPolyline(data&)); }

rpavlik commented 2 years ago

yeah, iirc I tried to turn off as many copy constructors as possible in favor of move constructors, because it wasn't entirely clear to me that the memory management was being handled right. Can you point me to the code you tested with so I can see what's going on?

TFreudi1 commented 2 years ago

It is simple there is the dwg2dxf project on the same github site. Try to compile it.Von meinem/meiner Galaxy gesendet -------- Ursprüngliche Nachricht --------Von: "Ryan A. Pavlik" @.> Datum: 11.02.22 22:31 (GMT+01:00) An: LibreCAD/libdxfrw @.> Cc: TFreudi1 @.>, Comment @.> Betreff: Re: [LibreCAD/libdxfrw] Recommended upstream? (#22) yeah, iirc I tried to turn off as many copy constructors as possible in favor of move constructors, because it wasn't entirely clear to me that the memory management was being handled right. Can you point me to the code you tested with so I can see what's going on?

—Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you commented.Message ID: @.***>

TFreudi1 commented 2 years ago

https://github.com/LibreCAD/libdxfrw/tree/master/dwg2dxf