flameshot: init at 0.5.0 #34096
flameshot: init at 0.5.0 #34096
Conversation
| # flameshot.pro assumes qmake is being run in a git checkout and uses it | ||
| # to determine the version being built. Let's replace that. | ||
| "VERSION=${version}" | ||
| "PREFIX=/" |
There was a problem hiding this comment.
Cargo culted. Not sure if this is idiomatic.
| sed -i 's,USRPATH = /usr/local,USRPATH = /,g' flameshot.pro | ||
| ''; | ||
|
|
||
| installFlags = [ "INSTALL_ROOT=$(out)" ]; |
There was a problem hiding this comment.
Cargo culted. Not sure if this is idiomatic.
| ]; | ||
| patchPhase = '' | ||
| sed -i 's/VERSION =/#VERSION =/g' flameshot.pro | ||
| sed -i 's,USRPATH = /usr/local,USRPATH = /,g' flameshot.pro |
There was a problem hiding this comment.
This feels ugly, but not sure if there's a cleaner way. flameshot.pro contains hard-coded USRPATH that can take one of two values depending on whether CONFIG contains packaging.
There was a problem hiding this comment.
For convenience, here's flameshot.pro: https://github.com/lupoDharkael/flameshot/blob/v0.5.0/flameshot.pro
| description = "Powerful yet simple to use screenshot software"; | ||
| homepage = https://github.com/lupoDharkael/flameshot; | ||
| license = stdenv.lib.licenses.gpl3; | ||
| platforms = stdenv.lib.platforms.all; |
There was a problem hiding this comment.
As indicated in the PR I have only tested this on Ubuntu. From grepping around, i get the impression that .all would be appropriate in this case anyway.
|
Looks pretty good, can you:
@GrahamcOfBorg build flameshot |
There was a problem hiding this comment.
Success for system: x86_64-linux
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/8ivhdhqdzkz0nc1wgpd98rdipqfcsd7j-flameshot-0.5.0
shrinking /nix/store/8ivhdhqdzkz0nc1wgpd98rdipqfcsd7j-flameshot-0.5.0/bin/flameshot
strip is /nix/store/mdyy001q67hiks0g24ra53z7ckm4jfr4-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/8ivhdhqdzkz0nc1wgpd98rdipqfcsd7j-flameshot-0.5.0/bin
patching script interpreter paths in /nix/store/8ivhdhqdzkz0nc1wgpd98rdipqfcsd7j-flameshot-0.5.0
checking for references to /tmp/nix-build-flameshot-0.5.0.drv-0 in /nix/store/8ivhdhqdzkz0nc1wgpd98rdipqfcsd7j-flameshot-0.5.0...
postPatchMkspecs
postPatchMkspecs
/nix/store/8ivhdhqdzkz0nc1wgpd98rdipqfcsd7j-flameshot-0.5.0
There was a problem hiding this comment.
Success for system: aarch64-linux
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/qqi1xjcib3lhm1cj3lf4a9321jb7znh8-flameshot-0.5.0
shrinking /nix/store/qqi1xjcib3lhm1cj3lf4a9321jb7znh8-flameshot-0.5.0/bin/flameshot
strip is /nix/store/jwz859pxqj7sl2dbwvmxkx68jp774izb-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/qqi1xjcib3lhm1cj3lf4a9321jb7znh8-flameshot-0.5.0/bin
patching script interpreter paths in /nix/store/qqi1xjcib3lhm1cj3lf4a9321jb7znh8-flameshot-0.5.0
checking for references to /build in /nix/store/qqi1xjcib3lhm1cj3lf4a9321jb7znh8-flameshot-0.5.0...
postPatchMkspecs
postPatchMkspecs
/nix/store/qqi1xjcib3lhm1cj3lf4a9321jb7znh8-flameshot-0.5.0
There was a problem hiding this comment.
Failure for system: x86_64-darwin
^~~~~~~~~~
src/utils/screengrabber.cpp:51:10: note: implicit use of 'this' pointer is only allowed within the evaluation of a call to a 'constexpr' member function
src/utils/screengrabber.cpp:38:8: error: case value is not a constant expression
case m_info.GNOME: {
^~~~~~~~~~~~
src/utils/screengrabber.cpp:38:8: note: implicit use of 'this' pointer is only allowed within the evaluation of a call to a 'constexpr' member function
2 errors generated.
make: *** [Makefile:2074: screengrabber.o] Error 1
builder for '/nix/store/jlahkn045lp2p0mf2zr0r3hdkrkagnwy-flameshot-0.5.0.drv' failed with exit code 2
error: build of '/nix/store/jlahkn045lp2p0mf2zr0r3hdkrkagnwy-flameshot-0.5.0.drv' failed
scode
force-pushed
the
scode/flameshot
branch
from
5496177 to
f083944
Compare
|
@grahamc Done. Apologies. I'm so used to a squash based workflow that I just pushed another commit without thinking. Also fixed the commit msg. |
|
Not sure why it's failing like this on Darwin since I'd expect the compilers to be the same. |
|
darwin uses clang, while on linux we have gcc. |
|
Reproed locally with clang. I'll submit a PR upstream. However, curious what the general policy is for things like these. Other than of course preferring to make upstream portable, would the strategy be to let it go in and not work on darwin? Or change the platform to just include linux? Or go ahead and provide patches in nix? |
As encountered in NixOS/nixpkgs#34096 we fail to build under clang: ``` src/utils/screengrabber.cpp:56:16: error: case value is not a constant expression ```
|
flameshot-org/flameshot#101 for the upstream fix |
|
FWIW, upstream is merged (but there's no release containing it yet). Thoughts on best path forward? |
Janik-Haag
added
the
12. first-time contribution
Motivation for this change
flameshot is very useful, let's add it!
Things done
(tested on Ubuntu)
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)