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.

Sign up for GitHub

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

friction-graphics: init at 0.9.6 #322957

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

friction-graphics: init at 0.9.6 #322957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6220b9f
maintainers: add socksy
socksy Jun 27, 2024
1809c70
friction-graphics: init at 0.9.6.1
socksy Jun 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions maintainers/maintainer-list.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20618,6 +20618,12 @@
githubId = 55726;
name = "Stanislav Ochotnický";
};
socksy = {
email = "[email protected]";
github = "socksy";
githubId = 511549;
name = "Ben Lovell";
};
sodiboo = {
name = "sodiboo";
github = "sodiboo";
Expand Down
95 changes: 95 additions & 0 deletions pkgs/by-name/fr/friction-graphics/package.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
{
lib,
clangStdenv,
cmake,
expat,
fetchgit,
ffmpeg_4-full,
fontconfig,
freetype,
glib,
gn,
harfbuzzFull,
icu,
libGL,
libjpeg_turbo,
libpng,
libsForQt5,
libunwind,
libwebp,
ninja,
pcre2,
pkg-config,
python3,
qt5Full,
xorg,
zlib,
}:

clangStdenv.mkDerivation rec {
pname = "friction.graphics";
version = "0.9.6.1";
src = fetchgit {
Copy link
Member

Choose a reason for hiding this comment

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

Please use fetchFromGitHub.
rev has to be "refs/tags/v${version}"

url = "https://github.com/friction2d/friction";
rev = version;
sha256 = "sha256-Z0xVEj9tO5TxNXWjzoauk8HaCLMOMkoTsq3+UYlLr9k=";
fetchSubmodules = true;
};
buildInputs = [
expat
ffmpeg_4-full
fontconfig
freetype
glib
gn
harfbuzzFull
icu
libGL
libjpeg_turbo
libpng
libsForQt5.qscintilla
libunwind
libwebp
ninja
socksy marked this conversation as resolved.
Show resolved Hide resolved
pcre2
python3
qt5Full
xorg.libX11
zlib
];
nativeBuildInputs = [
cmake
pkg-config
];
cmakeFlags = [
"-DCMAKE_BUILD_TYPE=Release"
"-DCMAKE_INSTALL_PREFIX=bin"
"-DHARFBUZZ_INCLUDE_DIRS=${harfbuzzFull.dev}/include"
"-DQSCINTILLA_INCLUDE_DIRS=${libsForQt5.qscintilla}/include"
"-DQSCINTILLA_LIBRARIES_DIRS=${libsForQt5.qscintilla}/lib/"
"-DQSCINTILLA_LIBRARIES=libqscintilla2.so"
"-G Ninja"
];
patchPhase = ''
sed -i 's|''${SKIA_SRC}/bin/gn)|${gn}/bin/gn)|' src/engine/CMakeLists.txt
sed -i 's|fontconfig|${fontconfig.lib}/lib/libfontconfig.so|' src/cmake/friction-common.cmake
grep -rl 'hb' | xargs sed -Ei 's/(["<])(hb.*\.h)/\1harfbuzz\/\2/'
'';
Comment on lines +73 to +77
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
patchPhase = ''
sed -i 's|''${SKIA_SRC}/bin/gn)|${gn}/bin/gn)|' src/engine/CMakeLists.txt
sed -i 's|fontconfig|${fontconfig.lib}/lib/libfontconfig.so|' src/cmake/friction-common.cmake
grep -rl 'hb' | xargs sed -Ei 's/(["<])(hb.*\.h)/\1harfbuzz\/\2/'
'';
postPatch = ''
sed -i 's|''${SKIA_SRC}/bin/gn)|${gn}/bin/gn)|' src/engine/CMakeLists.txt
sed -i 's|fontconfig|${fontconfig.lib}/lib/libfontconfig.so|' src/cmake/friction-common.cmake
grep -rl 'hb' | xargs sed -Ei 's/(["<])(hb.*\.h)/\1harfbuzz\/\2/'
'';

postPatch is usually placed right after src.
Also please use subtituteInPlace with --replace-fail if possbible so that we easily notice when things no longer match

Copy link
Author

Choose a reason for hiding this comment

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

I couldn't work out if the last one was possible to change to substituteInPlace. It's replacing all #include <hb.h> and #include "hb.h" to #include <harfbuzz/hb.h>, since it seems this is where the harfbuzz that's how to get the header in harfbuzzFull. I'm not sure if it's too bad if this doesn't loudly fail though — it would mean that <hb.h> is no longer being referenced.

I updated the first two however, thanks for the advice 👍

buildPhase = ''
export VERBOSE=1
cmake --build . --config Release
Comment on lines +79 to +80
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export VERBOSE=1
cmake --build . --config Release
cmake --build . --config Release

Please use cmakeFlags to pass --config Release if necessary

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure if that's possible, this is what I get when I try to do that:
2024-07-20-011255_hyprshot

It's a flag that's explicitly documented in their README so I assumed it was necessary

'';
installPhase = ''
mkdir -p $out/bin
cp src/app/friction $out/friction
'';
Comment on lines +78 to +85
Copy link
Member

Choose a reason for hiding this comment

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

Please use the standard cmake hook if possible

Copy link
Author

Choose a reason for hiding this comment

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

I am not so familiar with cmake in general, is this something that's provided out of the box that is preferred over cp? I can't find anything related to installing in any of the *.cmake or CMakeLists.txt files. Installing isn't documented in the repo.

I looked around in other packages to see what they were doing for other cmake projects, and had found at least some use cp in this fashion, hence I followed it. I guess I could use GNU install instead? I'm a bit confused by this part in general...

Copy link
Member

Choose a reason for hiding this comment

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

Upstream should have install targets for those files and then they should be installed automatically by make install which gets generated from the cmake files.

meta = with lib; {
description = "Vector motion graphics program";
socksy marked this conversation as resolved.
Show resolved Hide resolved
longDescription = "Friction is a powerful and versatile motion graphics application that allows you to create stunning vector and raster animations for web and video platforms with ease.";
homepage = "https://friction.graphics/";
license = lib.licenses.gpl3;
platforms = lib.platforms.unix;
maintainers = with lib.maintainers; [ socksy ];
mainProgram = "friction";
};
}