On Sat, Jul 15, 2017 at 02:11:42PM +0200, Eduard Braun wrote:
Hi Bryce,
thanks for looking into this! I'm all in favour of tidying the repo. It's always hard to tell what's still needed and what can be (re)moved, especially with Inkscape having a lot of independent developers and I guess most people prefer to err on the safe of caution in order not to break anything for somebody else so a lot accumulates over time...
(@ Sebatian: Do we still need msysenv.sh, see below?)
Am 15.07.2017 um 01:41 schrieb Bryce Harrington:
- inkscape.ico
- inkscape.png
The share/icons/application/ directory is the formal place for our icons, with share/branding/ the official source for branding files.
Only the windows packaging refers to inkscape.ico. So I think this could be moved to packaging/. Or perhaps windows could be changed to use the .png files like everything else?
Doxyfile seems to reference inkscape.png but I believe it could as easily use one of the icons in share/icons/application/, but see below...
inkscape.ico is also compiled into Windows executables, so we can't drop it. Moving to share/icons/application/ would be fine though.
Sounds good.
- mingwenv.bat
- mingwenv.cmake
- msys2checkdeps.py
- msys2installdeps.sh
- msysenv.sh
- inkscape.appdata.xml.in
- Info.plist.in
- appveyor.sh
- appveyor.yml
- .gitlab-ci.yml
- .snapcraft.yaml
- packaging/
I know a lot of tools expect config files in the project's root directory, but can any of these be moved to packaging/? (Or maybe a utils/ dir?)
Several of these have description snippets that are essentially the first few paragraphs of the README. If the README is revised (see below), it may be worthwhile to resync the text in these from that.
What about moving (most of them) to a "buildtools" folder (I guess that describes them best)?
That would be fine with me. I almost suggested something similar myself but worried anything 'buildfoo' might get confused with cmake's 'build' directory. (I suggested 'utils' only because we have a similar directory in inkscape-web for misc. tools.)
mingwenv.bat - still used for devlibs-builds. Seeing that devlibs-builds are currently broken the best path is probably to completely move to MSYS2 eventually (unless somebody plans to continue to maintain the devlibs) which would make this file unneeded.
ok
mingwenv.cmake - should go into CMakeScripts/ (it's used by cmake when building on Windows)
That sounds good. Yes, there seems little reason not to continue consolodating cmake items under CMakeScripts.
msys2* - buildtools/ (CI relies on their location though, so has to be adjusted by whoever does the move) msysenv.sh - equivalent to mingwenv.bat when building from an MSYS shell. Is/was anybody ever using that? What about Sebastian (he introduced this file originally). Do you still need it? appveyor.sh - / buildtools (CI relies on it's location though, so has to be adjusted by whoever does the move) appveyor.yml - needs to stay in root (used by AppVeyor.ci). If you prefer to bunch everything it could be prefixed with a dot though
Yes, if there's stuff that simply can't be put into a subdir, but can be named starting with a dot, do so.
- download-gtest.sh
This still seems necessary for setting up gtest (although current trunk breaks on configure due to lack of a "Findgtest.cmake"; but that seems unrelated).
-- I figured this out, just a missing dependency; the gtest warning was just way more noticeable than the dependency error message. :-)
Having to use this script has always seemed a bit hacky to me. (and it's downloading gtest 1.7, but 1.8 is current). I know there's been some good work done on improving our testing cmake rules, I wonder if there is a way we can eliminate this script?
I added support for gtest 1.8 in 6b8520e which should now be auto-detected by cmake if installed in the system (tested on Windows, maybe somebody on Linux with gtest 1.8 installed could cross-check).
If we're fine to require gtest >= 1.8 (it's the first to include gmock) we can drop the script already (plus some legacy cmake code). Otherwise we'd require some additional code.
Yes, this definitely sounds like a situation where moving to the newer version simplifies the build. Since we're asking users to manually install it rather, instead of pulling it from the system, it's not an "ordinary" dependency so legacy support isn't a question. Besides, Inkscape 0.93 is already seeing a lot of major changes - gtk3, git, gitlab, cmake-only... changing the gtest version requirement is pretty minor in comparison.
Bryce