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

wxGUI: remove unused wxwidgets compilation configuration option #2593

Merged
merged 3 commits into from
Oct 9, 2022

Conversation

sisco0
Copy link
Contributor

@sisco0 sisco0 commented Oct 5, 2022

The configuration option wxwidget has been removed

Refs: #2578

The configuration option wxwidget has been removed

Refs: OSGeo#2578
@nilason
Copy link
Contributor

nilason commented Oct 5, 2022

This looks fine by me. --with-wxwidgets is indeed a dead-end configure option.

Just a small note: no need to change the macosx/ReadMe.md file at this point, it is in all totally outdated (as noted at the top of the file).

Thanks @sisco0 !

@ojwb
Copy link

ojwb commented Oct 5, 2022

I reported #2578 and @sisco0 asked me to check this patch works as expected, and it does. Thanks.

I did a quick grep in the tree to look for lingering references and I wonder if these can be removed too:

diff --git a/.github/workflows/osgeo4w.yml b/.github/workflows/osgeo4w.yml
index e645585d..4db5f0da 100644
--- a/.github/workflows/osgeo4w.yml
+++ b/.github/workflows/osgeo4w.yml
@@ -39,7 +39,7 @@ jobs:
           $exe = 'osgeo4w-setup.exe'
           $url = 'http://download.osgeo.org/osgeo4w/v2/' + $exe
           (New-Object System.Net.WebClient).DownloadFile($url, $exe)
-          Start-Process ('.\'+$exe) -ArgumentList '-A -g -k -q -s http://download.osgeo.org/osgeo4w/v2/ -P proj-devel,gdal-devel,geos-devel,libtiff-devel,libpng-devel,pdal-devel,netcdf-devel,cairo-devel,fftw,freetype-devel,gdal-ecw,gdal-mrsid,liblas-devel,libxdr,libpq-devel,pdcurses,python3-matplotlib,python3-numpy,python3-ply,python3-pywin32,python3-six,python3-wxpython,regex-devel,wxwidgets-devel,zstd-devel' -Wait
+          Start-Process ('.\'+$exe) -ArgumentList '-A -g -k -q -s http://download.osgeo.org/osgeo4w/v2/ -P proj-devel,gdal-devel,geos-devel,libtiff-devel,libpng-devel,pdal-devel,netcdf-devel,cairo-devel,fftw,freetype-devel,gdal-ecw,gdal-mrsid,liblas-devel,libxdr,libpq-devel,pdcurses,python3-matplotlib,python3-numpy,python3-ply,python3-pywin32,python3-six,python3-wxpython,regex-devel,zstd-devel' -Wait
 
       - name: Compile GRASS GIS
         run: D:\msys64\usr\bin\bash.exe -l (''+(Get-Location)+'\.github\workflows\build_osgeo4w.sh') (Get-Location)
diff --git a/.travis/linux.install.sh b/.travis/linux.install.sh
index 64022804..ffdbebcb 100755
--- a/.travis/linux.install.sh
+++ b/.travis/linux.install.sh
@@ -26,7 +26,6 @@ sudo apt-get install --no-install-recommends \
         libreadline-dev \
         libsqlite3-dev \
         libtiff-dev \
-        libwxgtk3.0-gtk3-dev \
         libxmu-dev \
         libzstd-dev \
         python3 \
diff --git a/Vagrantfile b/Vagrantfile
index 0a47b641..16499ba3 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -63,7 +63,6 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
       "proj-bin",
       "libreadline-dev",
       "libsqlite3-dev",
-      "libwxgtk3.0-gtk3-dev",
       "libxmu-dev",
       "python3",
       "python3-wxgtk4.0",

First hunk is dropping wxwidgets-devel, which I'm less sure about as I'm not familiar with whatever package system that is.

I'm much more confident about the libwxgtk3.0-gtk3-dev dependencies as I co-maintain those packages in Debian.

@sisco0 sisco0 marked this pull request as ready for review October 6, 2022 00:34
@sisco0
Copy link
Contributor Author

sisco0 commented Oct 6, 2022

The recommendations given by @ojwb have been considered and the PR has been moved to Review state.
MacOSX changes are kept with the intention to provide with an homogeneous solution.

Copy link
Contributor

@nilason nilason left a comment

Choose a reason for hiding this comment

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

Thanks!

Regarding the macos/ReadMe.md file, I'd prefer to keep its history untouched. Apart from being converted from rtf to md format, it's content was last modified in 2009! Rather a historical document, waiting for a major rewrite...

@nilason nilason requested a review from neteler October 6, 2022 07:08
Copy link
Member

@neteler neteler left a comment

Choose a reason for hiding this comment

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

Locally compiled with success, GUI works as expected.
Thanks for your contribution!

@nilason I'd suggest to backport it to G8.2

@neteler neteler added this to the 8.3.0 milestone Oct 6, 2022
The folder is discontinued and no updates are expected to be performed under it
@sisco0
Copy link
Contributor Author

sisco0 commented Oct 8, 2022

Thanks!

Regarding the macos/ReadMe.md file, I'd prefer to keep its history untouched. Apart from being converted from rtf to md format, it's content was last modified in 2009! Rather a historical document, waiting for a major rewrite...

@nilason, as per request, I got to provide with a last commit, which currently reverts the changes under the macos folder. Could you kindly take a look and see if it matches the expectations? Have a great day!

@nilason nilason merged commit 3c260ef into OSGeo:main Oct 9, 2022
nilason pushed a commit that referenced this pull request Oct 9, 2022
…2593)

The configuration option wxwidget has been removed

Refs: #2578
@nilason
Copy link
Contributor

nilason commented Oct 9, 2022

Merged and backported to 8.2.
Thank you @sisco0 !

ninsbl pushed a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
marisn pushed a commit to marisn/grass that referenced this pull request Jun 2, 2023
@neteler neteler changed the title fix: remove unused wxwidgets compilation configuration option wxGUI: remove unused wxwidgets compilation configuration option Jun 6, 2023
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants