Skip to content

php73: fix builds on darwin architecture#201

Merged
drupol merged 1 commit into
masterfrom
php73/fix-builds
Jan 25, 2023
Merged

php73: fix builds on darwin architecture#201
drupol merged 1 commit into
masterfrom
php73/fix-builds

Conversation

@drupol

@drupol drupol commented Jan 20, 2023

Copy link
Copy Markdown
Collaborator

This PR:

@HelloWorld017

Copy link
Copy Markdown
Contributor

I'll check whether this builds well on my machine

@HelloWorld017

HelloWorld017 commented Jan 20, 2023

Copy link
Copy Markdown
Contributor

This derivation also keeps failing with following issue:

PEAR package PHP_Archive not installed: generated phar will require PHP's phar extension be enabled.

The flag --with-pcre-regex=${prev.pcre2.dev} should be added in order to the php73 to be built.
Here's the commit:

$ git am - <<'EOF'
From ac13f9d341dc63bf366699cfc2911601b73a849b Mon Sep 17 00:00:00 2001
From: nenw <khi@nenw.dev>
Date: Fri, 20 Jan 2023 20:00:17 +0900
Subject: [PATCH] php73: fix pcre config flags

---
 pkgs/phps.nix | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pkgs/phps.nix b/pkgs/phps.nix
index 71a884d..760397e 100644
--- a/pkgs/phps.nix
+++ b/pkgs/phps.nix
@@ -43,6 +43,7 @@ let
 
               "--enable-libxml"
               "--with-libxml-dir=${prev.libxml2.dev}"
+              "--with-pcre-regex=${prev.pcre2.dev}"
             ];
 
           buildInputs =
-- 
2.38.1

EOF

@drupol
drupol force-pushed the php73/fix-builds branch 2 times, most recently from e5e7735 to ba7bf9c Compare January 20, 2023 16:27
@drupol

drupol commented Jan 20, 2023

Copy link
Copy Markdown
Collaborator Author

Can you try again now?

Comment thread pkgs/php/7.3.nix Outdated
@drupol
drupol force-pushed the php73/fix-builds branch 5 times, most recently from b3af256 to 0f749b8 Compare January 20, 2023 22:01
@drupol

drupol commented Jan 20, 2023

Copy link
Copy Markdown
Collaborator Author

I pushed a temporary commit (part of #85) to check if it is correctly building on MacOS.
I'll remove it when this PR has been approved, just before merging it.

Comment thread pkgs/patches/php73-libtool.patch Outdated
@drupol

drupol commented Jan 25, 2023

Copy link
Copy Markdown
Collaborator Author

@HelloWorld017 @shyim Any update here?

@shyim

shyim commented Jan 25, 2023

Copy link
Copy Markdown
Contributor
shyim in nix-phps on  php73/fix-builds [?] took 5m47s
❯ ./result/bin/php -v
PHP 7.3.28 (cli) (built: Jan 25 2023 08:29:17) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.3.28, Copyright (c) 1998-2018 Zend Technologies
    with Zend OPcache v7.3.28, Copyright (c) 1999-2018, by Zend Technologies

whooo works

@HelloWorld017

Copy link
Copy Markdown
Contributor

Yes, it works on my laptop too. 👍

@drupol

drupol commented Jan 25, 2023

Copy link
Copy Markdown
Collaborator Author

@jtojnar Should I remove the commit 2ca5c87 before merging? WDYT ?

@jtojnar

jtojnar commented Jan 25, 2023

Copy link
Copy Markdown
Member

Might be faster to merge it that way, since we would have to review that patch otherwise.

@drupol

drupol commented Jan 25, 2023

Copy link
Copy Markdown
Collaborator Author

I just removed the commit.

@drupol
drupol merged commit 7e53d72 into master Jan 25, 2023
@drupol
drupol deleted the php73/fix-builds branch January 25, 2023 22:17
Comment thread pkgs/phps.nix
"--with-libxml-dir=${prev.libxml2.dev}"
]
++ prev.lib.optionals (prev.lib.versionAtLeast args.version "7.3") [
"--with-pcre-regex=${prev.pcre2.dev}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Actually, not sure about this. Should not it be versionOlder args.version "7.4"? I would expect newer PHP versions to pick it up using pkg-config.

Copy link
Copy Markdown
Collaborator 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 either, maybe @shyim or @HelloWorld017 knows?

@HelloWorld017 HelloWorld017 Jan 26, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

He is right. It should be applied to php<=7.3 (7.3, 7.2, ...)

You can just add it below to the xml line

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Dammit...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'll open a new PR right now. Sorry about that.

drupol added a commit that referenced this pull request Jan 26, 2023
drupol added a commit that referenced this pull request Jan 26, 2023
drupol added a commit that referenced this pull request Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants