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

Codechange: Drop libxdg-basedir dependency in favour of finding the directories ourselves #8357

Merged
merged 1 commit into from Jan 2, 2021

Conversation

@LordAro
Copy link
Member

@LordAro LordAro commented Dec 6, 2020

The dependency is a bit of a pain, and iirc @matthijskooijman expressed interest in dropping the package from debian entirely anyway.

And given replacing the dependency takes so little effort anyway...

Ideally the whole Determine(Base)Path stuff would be replaced with something that didn't rely on a load of manual string manipulation, but that's for later...

@LordAro LordAro force-pushed the LordAro:remove-xdg-basedir-dep branch 4 times, most recently from 29e8d2e to 8deaf5a Dec 6, 2020
src/fileio.cpp Outdated Show resolved Hide resolved
src/fileio.cpp Outdated Show resolved Hide resolved
@LordAro LordAro force-pushed the LordAro:remove-xdg-basedir-dep branch from 8deaf5a to dd28786 Dec 6, 2020
src/fileio.cpp Outdated Show resolved Hide resolved
src/fileio.cpp Outdated Show resolved Hide resolved
@LordAro LordAro force-pushed the LordAro:remove-xdg-basedir-dep branch from dd28786 to 1032722 Dec 6, 2020
Copy link
Contributor

@matthijskooijman matthijskooijman left a comment

The dependency is a bit of a pain, and iirc @matthijskooijman expressed interest in dropping the package from debian entirely anyway.

I don't really recall this and could not find any evidence, but looking in Ubuntu, there's only a handful packages left using libxdg-basedir, so I guess it's on its way to removal anyway, so +1.

src/fileio.cpp Outdated Show resolved Hide resolved
@LordAro LordAro force-pushed the LordAro:remove-xdg-basedir-dep branch from 1032722 to fb5daaa Dec 8, 2020
src/fileio.cpp Outdated Show resolved Hide resolved
@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Dec 8, 2020

Something to consider:

WITH_PERSONAL_PATH is with CMake always set. That is to say, unless someone is really out to unset it, but all OSes have a default. So if we take that into account, the USE_XDG macro becomes easier, and I am sure some other parts of the code becomes easier to read too.

No need to do this in this PR. It just jumped out for me :)

src/fileio.cpp Outdated Show resolved Hide resolved
src/fileio.cpp Outdated Show resolved Hide resolved
src/fileio.cpp Show resolved Hide resolved
src/fileio.cpp Outdated Show resolved Hide resolved
src/fileio.cpp Show resolved Hide resolved
@LordAro LordAro force-pushed the LordAro:remove-xdg-basedir-dep branch 2 times, most recently from c5b7cfe to 29b4b75 Jan 2, 2021
src/fileio.cpp Outdated Show resolved Hide resolved
src/fileio.cpp Outdated Show resolved Hide resolved
…irectories ourselves
@LordAro LordAro force-pushed the LordAro:remove-xdg-basedir-dep branch from 29b4b75 to fab9405 Jan 2, 2021
Copy link
Member

@TrueBrain TrueBrain left a comment

Couldn't find anything else to bitch about :(

@LordAro LordAro merged commit 3dfee97 into OpenTTD:master Jan 2, 2021
8 checks passed
8 checks passed
Emscripten
Details
Commit checker
Details
Check preview needs update Check preview needs update
Details
Linux (clang, clang++)
Details
Linux (gcc, g++)
Details
Mac OS (x64, x86_64) Mac OS (x64, x86_64)
Details
Windows (x86) Windows (x86)
Details
Windows (x64) Windows (x64)
Details
@LordAro LordAro deleted the LordAro:remove-xdg-basedir-dep branch Jan 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants