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

Nix support and use #1168

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft

Nix support and use #1168

wants to merge 18 commits into from

Conversation

AwesomeQubic
Copy link

Tested on NixOs and Arch

@askmeaboutlo0m
Copy link
Member

I managed to get this building on Arch. There's a few issues I found.

Major issue: It looks like it always rebuilds the whole thing from scratch for every tiny change when using nix build. Even a change that's totally irrelevant, like adding a line to the README. Is there a way around this? I can't find any documentation in that regard. If not, then it's useless for development, which means I don't see a reason to include this stuff into the repository at all, since if you want to get a release build, you can use something from nixpkg.

It always builds against Qt6 in release mode with LTO enabled with no server (which means also no "host on this computer") and no tools. For development, you want to be able to build in debug mode, against Qt5 and in different configurations.

The build seems to spew a result directory into the tree. So /result/ should be added to the .gitignore to avoid accidentally committing that.

The build doesn't seem to run in the git repository, which means the version detection doesn't doesn't work. There's also a bug in the cmake scripts that causes the fallback version to not be set, so currently it ends up with a blank version, but that's fixed in 3eeea57. It would be nicer if it used the proper version, which could be passed explicitly using -DBUILD_VERSION=$(git describe --dirty), but this is not critical.

Also not that important: Qt whinges about the locale not being UTF-8 when starting up Drawpile. It fixes that on its own, but it would be nicer to not have a goofy locale in the first place to avoid the warning. I dunno if that's an issue that can be fixed in the flake or if that's a problem with nix itself.

Also nix flake check fails. It installs fine regardless, but this is probably something that should be fixed?

Very small annoyance: there's trailing whitespace in flake.nix and a missing line feed at the end of default.nix and shell.nix. Git warns about these and when someone with a properly configured editor edits those files, they'll get stray changes in their diffs because of this. You should fix your editor configuration so that it strips errant whitespace at the end of lines and properly terminates the last line of files with a line feed.

Drawpile's repository doesn't allow merge commits and I'm not sure how well GitHub would handle rebasing on its own, so ideally do that locally to get your branch updated, rather than doing merges. It should just be:

# Assuming you have the official Drawpile remote as "drawpile"
git fetch drawpile
git rebase drawpile/main
# The log should now show that your commit is on top of Drawpile's main, without any merge commits
git log
# If that worked, overwrite the remote branch.
git push -f

That should just get rid of those "Merge branch…" commits and instead just put your commit(s) on top of the current state cleanly.

@askmeaboutlo0m askmeaboutlo0m self-requested a review December 3, 2023 09:21
Copy link
Member

@askmeaboutlo0m askmeaboutlo0m left a comment

Choose a reason for hiding this comment

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

See comment above, GitHub wants me to write another.

@AwesomeQubic AwesomeQubic marked this pull request as draft December 4, 2023 22:41
@AwesomeQubic AwesomeQubic changed the title Nix: Add basic nix flake Nix support and use Dec 4, 2023
@AwesomeQubic
Copy link
Author

Here is the v2 nix flake:
for qt5 shell: nix develop .#qt5-shell
for qt6 shell: nix develop .#qt6-shell

@MorrowShore MorrowShore added the stale Inactive issues about to be closed label Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Inactive issues about to be closed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants