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

Simplify KRB5 recipe #5

Merged
merged 6 commits into from
Jun 5, 2024
Merged

Simplify KRB5 recipe #5

merged 6 commits into from
Jun 5, 2024

Conversation

uilianries
Copy link

Specify library name and version: krb5/1.21.2

@Jihadist Thank you for your effort doing this PR to ConanCenterIndex. As it would many changes, I prefer sending you a PR with the suggestion instead of commenting in the PR conan-io#16074.

Some important point that I observed and changed in your PR:

  • Updated to the latest version 1.21.2
  • As recipe is not working yet on Windows, neither in MacOS, I removed all support. Let's keep it simple first, then improve later.
  • The static library requires extra patch affecting the source code. It's something that should be reported in the upstream first. ConanCenter can not afford maintaining non-official patches. Let's keep the recipe as shared-library for now, so people can consume it without invalid configuration
  • The Openssl provided by Conan uses zlib enabled by default, krb5 uses find library instead of searching for modules. I sent a PR to the upstream asking to fix it: Search OpenSSL using pkg-config instead of checking for libraries krb5/krb5#1351
  • The test_v1_package is no needed for new recipes. We are avoiding maintaining legacy generators.
  • The self.env_info is also no longer supported for new recipes.

Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
@xakod
Copy link
Owner

xakod commented Jun 5, 2024

@uilianries thank you for this changes!

  • If latest version works it would be great and we should use it. I didn't check build with 1.21.2.
  • Ok, I'm agree with you
  • Ok
  • Thank you again here :)
  • Ok
  • Nice, I didn't know

@xakod xakod merged commit dd326f6 into xakod:krb5 Jun 5, 2024
6 checks passed
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.

2 participants