unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#55541] [PATCH] gnu: Add azpainter.
@ 2022-05-20 11:18 Tobias Kortkamp
  2022-05-20 16:00 ` Maxime Devos
  2022-06-24 20:56 ` bug#55541: " Ludovic Courtès
  0 siblings, 2 replies; 4+ messages in thread
From: Tobias Kortkamp @ 2022-05-20 11:18 UTC (permalink / raw)
  To: 55541; +Cc: Tobias Kortkamp

* gnu/packages/graphics.scm (azpainter): New variable.
---
 gnu/packages/graphics.scm | 66 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/gnu/packages/graphics.scm b/gnu/packages/graphics.scm
index e966a82dbb..1457cf83fb 100644
--- a/gnu/packages/graphics.scm
+++ b/gnu/packages/graphics.scm
@@ -30,6 +30,7 @@
 ;;; Copyright © 2022 Michael Rohleder <mike@rohleder.de>
 ;;; Copyright © 2022 John Kehayias <john.kehayias@protonmail.com>
 ;;; Copyright © 2022 Zheng Junjie <873216071@qq.com>
+;;; Copyright © 2022 Tobias Kortkamp <tobias.kortkamp@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -2124,3 +2125,68 @@ (define-public monado
 such as VR and AR on mobile, PC/desktop, and any other device.  Monado aims to be
 a complete and conforming implementation of the OpenXR API made by Khronos.")
     (license license:boost1.0)))
+
+(define-public azpainter
+  (package
+    (name "azpainter")
+    (version "3.0.5")
+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://gitlab.com/azelpg/azpainter")
+                    (commit (string-append "v" version))))
+              (file-name (git-file-name name version))
+              (sha256
+               (base32
+                "1iplp3p8pw9q44kb43hrk89sv2aff6bdy9fk58j2v6k5lqbk6kvf"))))
+    (build-system gnu-build-system) ;actually a home grown build system
+    (arguments
+     (list #:tests? #f
+           #:phases
+           #~(modify-phases %standard-phases
+               (replace 'configure
+                 (lambda _
+                   (invoke "./configure"
+                           (string-append "--prefix="
+                                          #$output))))
+               (replace 'build
+                 (lambda* (#:key parallel-build? #:allow-other-keys)
+                   (let ((job-count (if parallel-build?
+                                        (number->string (parallel-job-count))
+                                        "1")))
+                     (invoke "ninja" "-j" job-count "-C" "build"))))
+               (add-before 'install 'disable-cache-generation
+                 (lambda _
+                   (setenv "DESTDIR" "/") #t))
+               (replace 'install
+                 (lambda _
+                   (invoke "ninja" "-C" "build" "install"))))))
+    (inputs (list fontconfig
+                  freetype
+                  libjpeg-turbo
+                  libpng
+                  libtiff
+                  libwebp
+                  libx11
+                  libxcursor
+                  libxext
+                  libxi
+                  zlib))
+    (native-inputs (list ninja pkg-config))
+    (home-page "http://azsky2.html.xdomain.jp/soft/azpainter.html")
+    (synopsis "Paint software for editing illustrations and images")
+    (description
+     "AzPainter is a lightweight full color painting application for editing
+illustrations and images.
+
+Features include:
+@itemize
+@item Layers
+@item Many artistic filters
+@item Good range of selection tools
+@item Pen pressure support with automatic brush size adjustment
+@item Support for 16-bit color images with transparency (RGBA)
+@item Support for image formats like PSD, PNG, JPEG, TIFF, WebP
+@end itemize
+")
+    (license license:gpl3+)))

base-commit: 2f170893719e6e9fc8e19cc5f0568e20a95d92b4
-- 
2.36.0





^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [bug#55541] [PATCH] gnu: Add azpainter.
  2022-05-20 11:18 [bug#55541] [PATCH] gnu: Add azpainter Tobias Kortkamp
@ 2022-05-20 16:00 ` Maxime Devos
  2022-06-24 20:56 ` bug#55541: " Ludovic Courtès
  1 sibling, 0 replies; 4+ messages in thread
From: Maxime Devos @ 2022-05-20 16:00 UTC (permalink / raw)
  To: Tobias Kortkamp, 55541

[-- Attachment #1: Type: text/plain, Size: 1987 bytes --]

Tobias Kortkamp schreef op vr 20-05-2022 om 13:18 [+0200]:
> +    (build-system gnu-build-system) ;actually a home grown build system
> +    (arguments
> +     (list #:tests? #f
> +           #:phases
> +           #~(modify-phases %standard-phases
> +               (replace 'configure
> +                 (lambda _
> +                   (invoke "./configure"
> +                           (string-append "--prefix="
> +                                          #$output))))

As-is, this home-grown build system is broken when cross-compiling:

  * When cross-compiling, TARGET-gcc needs to be used instead of gcc.
    Maybe do (setenv "CC" #$(cc-for-target)) first?

  * Likewise, TARGET-pkg-config instead of pkg-config (not 100% sure)

  * It tries to run binaries during ./configure.  When cross-compiling,
    ./conftest will always fail (unless using emulation) and hence
    always detect ‘little endian’ but this is incorrect when
    cross-compiling for big-endian architectures.

(Needs some fixes or work-arounds.)  You can test with "guix build
azpainter --target=aarch64-linux-gnu" or such.

Also, some other problems.  From mlk_studio.c

int mFILEreadBE32(FILE *fp,void *buf)
{
	uint8_t v[4];

	if(fread(v, 1, 4, fp) < 4)
		return 1;
	else
	{
		*((uint32_t *)buf) = ((uint32_t)v[0] << 24) | (v[1] <<
16) | (v[2] << 8) | v[3];
		return 0;
	}
}

looks like a potential strict-aliasing violation to me, resulting in
undefined behaviour -- what if buf is a pointer to an array of, say,
doubles?  Also a potential alignment problem, though maybe it's only
called for sufficiently aligned 'buf'.  The strict-aliasing problem
can be worked around with -fno-strict-aliasing or maybe just -fno-ipa-
strict-aliasing , though I don't know if that's sufficient.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* bug#55541: [PATCH] gnu: Add azpainter.
  2022-05-20 11:18 [bug#55541] [PATCH] gnu: Add azpainter Tobias Kortkamp
  2022-05-20 16:00 ` Maxime Devos
@ 2022-06-24 20:56 ` Ludovic Courtès
  2022-06-24 21:41   ` [bug#55541] " Maxime Devos
  1 sibling, 1 reply; 4+ messages in thread
From: Ludovic Courtès @ 2022-06-24 20:56 UTC (permalink / raw)
  To: Tobias Kortkamp; +Cc: 55541-done

Hi Tobias,

Tobias Kortkamp <tobias.kortkamp@gmail.com> skribis:

> * gnu/packages/graphics.scm (azpainter): New variable.

Applied, thanks!

Maxime Devos <maximedevos@telenet.be> skribis:

> As-is, this home-grown build system is broken when cross-compiling:
>
>   * When cross-compiling, TARGET-gcc needs to be used instead of gcc.
>     Maybe do (setenv "CC" #$(cc-for-target)) first?
>
>   * Likewise, TARGET-pkg-config instead of pkg-config (not 100% sure)
>
>   * It tries to run binaries during ./configure.  When cross-compiling,
>     ./conftest will always fail (unless using emulation) and hence
>     always detect ‘little endian’ but this is incorrect when
>     cross-compiling for big-endian architectures.
>
> (Needs some fixes or work-arounds.)  You can test with "guix build
> azpainter --target=aarch64-linux-gnu" or such.
>
> Also, some other problems.  From mlk_studio.c
>
> int mFILEreadBE32(FILE *fp,void *buf)
> {
> 	uint8_t v[4];
>
> 	if(fread(v, 1, 4, fp) < 4)
> 		return 1;
> 	else
> 	{
> 		*((uint32_t *)buf) = ((uint32_t)v[0] << 24) | (v[1] <<
> 16) | (v[2] << 8) | v[3];
> 		return 0;
> 	}
> }
>
> looks like a potential strict-aliasing violation to me, resulting in
> undefined behaviour -- what if buf is a pointer to an array of, say,
> doubles?  Also a potential alignment problem, though maybe it's only
> called for sufficiently aligned 'buf'.  The strict-aliasing problem
> can be worked around with -fno-strict-aliasing or maybe just -fno-ipa-
> strict-aliasing , though I don't know if that's sufficient.

These are all good points and I appreciate that you did such a thorough
review (audit?) of the package!

That said, I think it’s a bit too much to ask of a downstream packager
or user to address these issues.  As I see it, these issues should be
reported upstream and addressed upstream.

I hope that makes sense!

Thanks,
Ludo’.




^ permalink raw reply	[flat|nested] 4+ messages in thread

* [bug#55541] [PATCH] gnu: Add azpainter.
  2022-06-24 20:56 ` bug#55541: " Ludovic Courtès
@ 2022-06-24 21:41   ` Maxime Devos
  0 siblings, 0 replies; 4+ messages in thread
From: Maxime Devos @ 2022-06-24 21:41 UTC (permalink / raw)
  To: Ludovic Courtès, Tobias Kortkamp; +Cc: 55541

[-- Attachment #1: Type: text/plain, Size: 2056 bytes --]

reopen 55541

Ludovic Courtès schreef op vr 24-06-2022 om 22:56 [+0200]:
> These are all good points and I appreciate that you did such a thorough
> review (audit?) of the package!

I looked through the code a bit, didn't check every file, so I wouldn't
call it an audit.

> 
> That said, I think it’s a bit too much to ask of a downstream packager
> or user to address these issues.  As I see it, these issues should be
> reported upstream and addressed upstream.
> 
> I hope that makes sense!

AFAICT the issues have not been reported upstream yet, so I don't think
we can close this entry on debbugs yet.  While I'd like for downstream
packaging to be trivial, the sad reality is that sometimes is not the
case, the issues are still there and need to be resolved somehow (fixed
downstream or upstream, or reported upstream).

If not by the new downstream packager that submitted the patch, then by
the the one committing the patch, or by a reviewer, or by some more
neboluous role of a random Guix contributor, or in some exceptional
cases the issue could be considered ‘too difficult and not too bad’
with some corresponding reasoning.  (It's most efficient if the
reporting or fixing is done directly by the submitter, but if the
submitter can't do it for whatever reason, then surely something can
eventually be worked out by other people, albeit more slowly.)

However, AFAICT, none of that has happened yet.

More generally, I don't think we should have an ‘packages included in
Guix should be good, unless submitted by a newbie’ exception.  Also,
potentially the new submitter would _like_ to learn more about Guix
(and have time for it, etc.) and learn how to improve things?

In the future, if someone submits a patch and I notice it has some
complicated problems, should I just ignore the complicated problems and
just LGTM?  This seems contrary to the concept of reviewing to me. 
(This is probably not what you meant, but to me, this is implied by
your response.)

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-06-24 21:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-20 11:18 [bug#55541] [PATCH] gnu: Add azpainter Tobias Kortkamp
2022-05-20 16:00 ` Maxime Devos
2022-06-24 20:56 ` bug#55541: " Ludovic Courtès
2022-06-24 21:41   ` [bug#55541] " Maxime Devos

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).