Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v.in.dwg: Retire v.in.dwg infavor of v.in.redwg #4329

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

ymdatta
Copy link
Contributor

@ymdatta ymdatta commented Sep 16, 2024

v.in.dwg is linked to proprietary library 'OpenDWG toolkit'. One needs to become a member of OpenDWG Alliance to get the needed libraries and headers in order to compile this module.

However, OpenDWG webpage went offline around 2011 and we have 'v.in.redwg' as replacement for it. Hence retire 'v.in.dwg' module.

@echoix
Copy link
Member

echoix commented Sep 16, 2024

You also need to remove the entry in the Makefile of the parent folder,

v.in.dwg \

@github-actions github-actions bot added vector Related to vector data processing C Related code is in C HTML Related code is in HTML module docs labels Sep 17, 2024
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this is even more complicated. Changes in configure.ac include/Make/Platform.make.in are needed. @ymdatta please do that and regenerate configure if you feel up to it or open an issue for it (and we merge this PR without the change).

@HamishB
Copy link
Contributor

HamishB commented Sep 18, 2024

Why not rename v.in.redwg as v.in.dwg and v.in.dwg as v.in.opendwg? The end user doesn't care what the backend library is, only that the name is self-documenting and it does what they want it to.

best,
Hamish

@wenzeslaus
Copy link
Member

wenzeslaus commented Sep 18, 2024

Why not rename v.in.redwg as v.in.dwg and v.in.dwg as v.in.opendwg?

Yes, that's possible. v.in.redwg is currently in grass-addons, so it an independent discussion from this PR and may include discussion about moving it to the core (perhaps you want to open a GitHub Discussion or grass-dev thread for that). v.in.opendwg is not bad in general, but I don't see someone actually maintaining the code, so keeping in active code base (in core or addons) does not seem good.

@echoix
Copy link
Member

echoix commented Sep 18, 2024

Renaming a module is kinda disruptive for anyone having scripted workflows. If the backend library changes, (thus keeping this module), and has somewhat a similar behavior as before, why would we need to rename the module?
The interface (options) might be similar.

It's reasonable to say like : this module now uses a new library, and might have some differences, but is more recent/accepts newer files types

Rather than: oh oh, that module doesn't exist anymore at all, change to use that same module with another name and library.

v.in.dwg is linked to proprietary library 'OpenDWG toolkit'. One
needs to become a member of OpenDWG Alliance to get the needed
libraries and headers in order to compile this module.

However, OpenDWG webpage went offline around 2011 and we have
'v.in.redwg' as replacement for it. Hence retire 'v.in.dwg' module.

Modify configure.ac and platform make to remove any remnants
of opendwg in them.

Signed-off-by: Mohan Yelugoti <ymdatta.work@gmail.com>
@ymdatta
Copy link
Contributor Author

ymdatta commented Sep 18, 2024

@wenzeslaus : Made the necessary changes.

  1. Removed opendwg remnants from configure.ac file.
  2. Regenerated configure script.
  3. Removed opendwg entry from include/Platform.make.in file.

Minor testing done: able to configure with current configuration and compile without any issues. When configure was run, didn't see any indication of dwg in the support section of config.

Earlier: (Indicated with asteriks around, Line 5)

  BLAS support:               no
  BZIP2 support:              no
  C++ support:                yes
  Cairo support:              yes
  **DWG support:                no**
  FFTW support:               yes
  FreeType support:           yes
  GDAL support:               yes
  GEOS support:               yes
  LAPACK support:             no
  Large File support (LFS):   yes
  libLAS support:             no
  LIBSVM support:             no
  MySQL support:              no
  NetCDF support:             no
  NLS support:                yes
  ODBC support:               no
  OGR support:                yes
  OpenCL support:             no
  OpenGL support:             yes
  OpenMP support:             yes
  PDAL support:               yes
  PNG support:                yes
  POSIX thread support:       no
  PostgreSQL support:         yes
  Readline support:           no
  Regex support:              yes
  SQLite support:             yes
  TIFF support:               yes
  X11 support:                yes
  Zstandard support:          yes

After the change:

  BLAS support:               no
  BZIP2 support:              no
  C++ support:                yes
  Cairo support:              yes
  FFTW support:               yes
  FreeType support:           yes
  GDAL support:               yes
  GEOS support:               yes
  LAPACK support:             no
  Large File support (LFS):   yes
  libLAS support:             no
  LIBSVM support:             no
  MySQL support:              no
  NetCDF support:             no
  NLS support:                yes
  ODBC support:               no
  OGR support:                yes
  OpenCL support:             no
  OpenGL support:             yes
  OpenMP support:             yes
  PDAL support:               yes
  PNG support:                yes
  POSIX thread support:       no
  PostgreSQL support:         yes
  Readline support:           no
  Regex support:              yes
  SQLite support:             yes
  TIFF support:               yes
  X11 support:                yes
  Zstandard support:          yes

@ymdatta ymdatta requested a review from wenzeslaus September 18, 2024 23:04
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. I can't find any other remains of OpenDWG.

@wenzeslaus wenzeslaus enabled auto-merge (squash) September 18, 2024 23:48
@wenzeslaus wenzeslaus merged commit 7dc46cd into OSGeo:main Sep 19, 2024
26 checks passed
@ymdatta ymdatta deleted the retire_vindwg branch September 19, 2024 02:51
@neteler neteler added this to the 8.5.0 milestone Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C Related code is in C docs HTML Related code is in HTML module vector Related to vector data processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants