gnu: Add fontconfig-path-max.

Message ID 7b313a7566d04932f94fb33e7a63c222@openmailbox.org
State New
Headers

Commit Message

rennes@openmailbox.org June 18, 2016, 7:02 p.m. UTC
  Hello Guix team,

i'm doing tests whith GNU Guix on GNU Hurd, compiling fontconfig and 
there is an error during compilation:

a) fontconfig uses the constant PATH_MAX.

Reviewing the documentation about the treatment of constant for Hurd; 
i've attached a patch for review.

References:
https://www.gnu.org/software/hurd/community/gsoc/project_ideas/maxpath.html
https://www.gnu.org/software/hurd/hurd/porting/guidelines.html

and i've a couple of questions about:

a) How Guix identify if it is a Linux or Hurd system at compile or 
install the package?.
b) i searches in ML an example, but i not found.



Thanks for your time.

Rene
  

Comments

Manolis Ragkousis July 3, 2016, 10:24 p.m. UTC | #1
Hello Rennes,

I am sorry for the long delay, I somehow missed the patch. Leo thank you
for telling me.

On 06/18/16 22:02, rennes@openmailbox.org wrote:
> Hello Guix team,
> 
> i'm doing tests whith GNU Guix on GNU Hurd, compiling fontconfig and
> there is an error during compilation:

Once again thank you :-)
> 
> a) fontconfig uses the constant PATH_MAX.
> 
> Reviewing the documentation about the treatment of constant for Hurd;
> i've attached a patch for review.
> 
> References:
> https://www.gnu.org/software/hurd/community/gsoc/project_ideas/maxpath.html
> https://www.gnu.org/software/hurd/hurd/porting/guidelines.html
> 
> and i've a couple of questions about:
> 
> a) How Guix identify if it is a Linux or Hurd system at compile or
> install the package?.
> b) i searches in ML an example, but i not found.

Currently we apply patches regardless of where it is running.

But one way to check the system is like in (gnu packages base)

(define* (glibc-for-target #:optional
                           (target (or (%current-target-system)
                                       (%current-system))))
  "Return the glibc for TARGET, GLIBC/LINUX for a Linux host or
GLIBC/HURD for a Hurd host"
  (match target
    ((or "i586-pc-gnu" "i586-gnu") glibc/hurd)
    (_ glibc/linux)))

If %current-target-system is not #f then we are cross-building for the
value inside it. %current-system has the value of the system we are
running on.

Now regarding the patch, what is the status on upstream? Are those
fontconfig patches present in fontconfig upstream?

Other than that, it looks good to me.

Thank you for helping on this,
Manolis.
  
rennes@openmailbox.org July 4, 2016, 4:02 a.m. UTC | #2
Hello,

thanks for the explanation.

> 
> Now regarding the patch, what is the status on upstream? Are those
> fontconfig patches present in fontconfig upstream?
> 

The current release is 2.12.0, and still uses the constant PATH_MAX.
And I have not found any related patch for this detail.

Thanks
  
Manolis Ragkousis July 4, 2016, 7:55 a.m. UTC | #3
Hello,

On 07/04/16 07:02, rennes@openmailbox.org wrote:
> The current release is 2.12.0, and still uses the constant PATH_MAX.
> And I have not found any related patch for this detail.

Could you send the related patches to fontconfig upstream and see what
they think about them?

Thank you,
Manolis
  
Ludovic Courtès July 4, 2016, 8:16 a.m. UTC | #4
Hello!

And thank you Manolis to taking care of this.  :-)

rennes@openmailbox.org skribis:

> From 3195bf1e75493675dc8cbd81a0f83e0b4538263b Mon Sep 17 00:00:00 2001
> From: Rene Saavedra <rennes@openmailbox.org>
> Date: Sat, 18 Jun 2016 13:37:19 -0500
> Subject: [PATCH] gnu: Add fontconfig-path-max.
>
> ---
>  gnu/packages/fontutils.scm                      |  3 +++
>  gnu/packages/patches/fontconfig-fcdefault.patch | 23 +++++++++++++++++++++++
>  gnu/packages/patches/fontconfig-fcstat.patch    | 23 +++++++++++++++++++++++
>  3 files changed, 49 insertions(+)
>  create mode 100644 gnu/packages/patches/fontconfig-fcdefault.patch
>  create mode 100644 gnu/packages/patches/fontconfig-fcstat.patch
>
> diff --git a/gnu/packages/fontutils.scm b/gnu/packages/fontutils.scm
> index 5f6ff15..2b84523 100644
> --- a/gnu/packages/fontutils.scm
> +++ b/gnu/packages/fontutils.scm
> @@ -231,6 +231,9 @@ fonts to/from the WOFF2 format.")
>              (uri (string-append
>                     "https://www.freedesktop.org/software/fontconfig/release/fontconfig-"
>                     version ".tar.bz2"))
> +            (patches (list
> +                      (search-patch "fontconfig-fcdefault.patch")
> +                      (search-patch "fontconfig-fcstat.patch")))

Rather use (search-patches …).

I would join the two patches in a single ‘fontconfig-path-max.patch’.

The file needs to be added in gnu/local.mk.

Manolis: do you think you could apply it to ‘core-updates-next’ with
these changes?

Thanks,
Ludo’.
  
Ludovic Courtès July 4, 2016, 8:21 a.m. UTC | #5
Oops, I spoke too fast.

rennes@openmailbox.org skribis:

> +++ b/gnu/packages/patches/fontconfig-fcdefault.patch
> @@ -0,0 +1,23 @@
> +This patch replaces the use of macro PATH_MAX by *buf constant,
> +which allows dynamic memory allocation.
> +
> +---
> + src/fcdefault.c | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/src/fcdefault.c b/src/fcdefault.c
> +index 6647a8f..8e2094f 100644
> +--- a/src/fcdefault.c
> ++++ b/src/fcdefault.c
> +@@ -150,7 +150,7 @@ retry:
> + # if defined (HAVE_GETEXECNAME)
> + 	const char *p = getexecname ();
> + # elif defined (HAVE_READLINK)
> +-	char buf[PATH_MAX + 1];
> ++	char *buf;

[...]

> + 	struct stat statb;
> +-	char f[PATH_MAX + 1];
> ++	char *f;

With these changes, the code compiles but will crash at run time,
because ‘f’ and ‘buf’ are dangling pointers.

We should instead use ‘01_path_max.patch’ from
<http://http.debian.net/debian/pool/main/f/fontconfig/fontconfig_2.11.0-6.3.debian.tar.xz>.

(In general, for PATH_MAX issues, Debian very likely already has a
patch.)

Thanks,
Ludo’.
  
rennes@openmailbox.org July 5, 2016, 1:46 a.m. UTC | #6
On 2016-07-04 02:55, Manolis Ragkousis wrote:
> Hello,
> 
> On 07/04/16 07:02, rennes@openmailbox.org wrote:
>> The current release is 2.12.0, and still uses the constant PATH_MAX.
>> And I have not found any related patch for this detail.
> 
> Could you send the related patches to fontconfig upstream and see what
> they think about them?
> 
> Thank you,
> Manolis

I will analyze and send the patches. For now I will use the Debian patch 
suggested by Ludo.

Thanks
  

Patch

From 3195bf1e75493675dc8cbd81a0f83e0b4538263b Mon Sep 17 00:00:00 2001
From: Rene Saavedra <rennes@openmailbox.org>
Date: Sat, 18 Jun 2016 13:37:19 -0500
Subject: [PATCH] gnu: Add fontconfig-path-max.

---
 gnu/packages/fontutils.scm                      |  3 +++
 gnu/packages/patches/fontconfig-fcdefault.patch | 23 +++++++++++++++++++++++
 gnu/packages/patches/fontconfig-fcstat.patch    | 23 +++++++++++++++++++++++
 3 files changed, 49 insertions(+)
 create mode 100644 gnu/packages/patches/fontconfig-fcdefault.patch
 create mode 100644 gnu/packages/patches/fontconfig-fcstat.patch

diff --git a/gnu/packages/fontutils.scm b/gnu/packages/fontutils.scm
index 5f6ff15..2b84523 100644
--- a/gnu/packages/fontutils.scm
+++ b/gnu/packages/fontutils.scm
@@ -231,6 +231,9 @@  fonts to/from the WOFF2 format.")
             (uri (string-append
                    "https://www.freedesktop.org/software/fontconfig/release/fontconfig-"
                    version ".tar.bz2"))
+            (patches (list
+                      (search-patch "fontconfig-fcdefault.patch")
+                      (search-patch "fontconfig-fcstat.patch")))
             (sha256 (base32
                      "1psrl4b4gi4wmbvwwh43lk491wsl8lgvqj146prlcha3vwjc0qyp"))))
    (build-system gnu-build-system)
diff --git a/gnu/packages/patches/fontconfig-fcdefault.patch b/gnu/packages/patches/fontconfig-fcdefault.patch
new file mode 100644
index 0000000..9c3b383
--- /dev/null
+++ b/gnu/packages/patches/fontconfig-fcdefault.patch
@@ -0,0 +1,23 @@ 
+This patch replaces the use of macro PATH_MAX by *buf constant,
+which allows dynamic memory allocation.
+
+---
+ src/fcdefault.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/src/fcdefault.c b/src/fcdefault.c
+index 6647a8f..8e2094f 100644
+--- a/src/fcdefault.c
++++ b/src/fcdefault.c
+@@ -150,7 +150,7 @@ retry:
+ # if defined (HAVE_GETEXECNAME)
+ 	const char *p = getexecname ();
+ # elif defined (HAVE_READLINK)
+-	char buf[PATH_MAX + 1];
++	char *buf;
+ 	int len;
+ 	char *p = NULL;
+ 
+-- 
+2.6.3
+
diff --git a/gnu/packages/patches/fontconfig-fcstat.patch b/gnu/packages/patches/fontconfig-fcstat.patch
new file mode 100644
index 0000000..e075b17
--- /dev/null
+++ b/gnu/packages/patches/fontconfig-fcstat.patch
@@ -0,0 +1,23 @@ 
+This patch replaces the use of macro PATH_MAX by *f constant,
+which allows dynamic memory allocation.
+
+---
+ src/fcstat.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/src/fcstat.c b/src/fcstat.c
+index 1734fa4..8a438eb 100644
+--- a/src/fcstat.c
++++ b/src/fcstat.c
+@@ -278,7 +278,7 @@ FcDirChecksum (const FcChar8 *dir, time_t *checksum)
+ 	{
+ #endif
+ 	struct stat statb;
+-	char f[PATH_MAX + 1];
++	char *f;
+ 
+ 	memcpy (f, dir, len);
+ 	f[len] = FC_DIR_SEPARATOR;
+-- 
+2.6.3
+
-- 
2.6.3