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.

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

ZSH - changed dependancy pcre->pcre2 since pcre is not compiling #26023

Closed
wants to merge 4 commits into from

Conversation

Behinder
Copy link

Description

Type(s)
  • [ X] bugfix
  • enhancement
  • security fix
Tested on

@macportsbot
Copy link

Notifying maintainers:
@larryv for port zsh.

@@ -21,7 +21,7 @@ homepage https://www.zsh.org
depends_lib port:gdbm \
port:libiconv \
port:ncurses \
port:pcre
port:pcre2
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing a library dependency requires a "revision" increase.

Copy link
Member

Choose a reason for hiding this comment

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

And also usually some change in configure flags or other way of controlling which libraries are used.

@reneeotten
Copy link
Contributor

reneeotten commented Sep 30, 2024

and your commit message doesn't follow our guidelines. It should be "portname: short description"; the port name isn't capitalized either.

@reneeotten
Copy link
Contributor

@Behinder please take care of the review comments and rebase with "masfer" to get rid of the merge conmit

@ryandesign
Copy link
Contributor

We are not aware that "pcre is not compiling"; if this is your experience, please file a bug report about it.

Copy link
Contributor

@reneeotten reneeotten left a comment

Choose a reason for hiding this comment

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

also, you didn't actually address @jmroot his comment about making sure that it will use/link with "pcre2".

@@ -49,7 +49,7 @@ checksums ${main_distfile} \
# See e.g. https://trac.macports.org/ticket/60419

# DO NOT change this unless you have understood and acted on the above comment!
set py_ver 3.10
set py_ver 3.12
Copy link
Contributor

Choose a reason for hiding this comment

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

did you read the comment above and made absolutely sure that nothing would break with this change?

Copy link
Member

Choose a reason for hiding this comment

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

Things will definitely break with that change, and it doesn't seem related to the rest of this PR.

@reneeotten reneeotten marked this pull request as draft October 13, 2024 15:12
@reneeotten reneeotten mentioned this pull request Oct 20, 2024
12 tasks
@reneeotten
Copy link
Contributor

reneeotten commented Oct 24, 2024

this PR is factually incorrect. If you look what the configure script does, it is looking for pcre.h which is a file that only is present in the pcre port (in the pcre2 port it is called pcre2.h). So the changes you suggest here will result in configure not finding pcre2 and compiling zsh without pcre support.
You can verify that this is the case yourself as you will see no mentions of pcre2 when doing:

otool -L `port contents zsh | grep  "\.so"` | grep pcre

whereas you will find linking to the pcre port in the presence of the pcre dependency.

Also, the pcre port is installing just fine here locally and also according to the status page it builds everywhere .

Therefore, closing this PR ; if you still have problems with the pcre port please file a Trac ticket.

@reneeotten reneeotten closed this Oct 24, 2024
@jmroot
Copy link
Member

jmroot commented Oct 24, 2024

FTR, it looks like the next upstream release of zsh will add pcre2 support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

6 participants