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

New package: Orthanc-1.11.2 #38537

Closed
wants to merge 6 commits into from
Closed

Conversation

gcarlos64
Copy link

Testing the changes

  • I tested the changes in this PR: briefly

New package

Local build testing

  • I built this PR locally for my native architecture, (x86_64-LIBC)

@abenson
Copy link
Contributor

abenson commented Aug 8, 2022

One commit per template

Copy link
Member

@ahesford ahesford left a comment

Choose a reason for hiding this comment

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

General comments (this is not exhaustive):

  • Are all of these shared libs really necessary for linking by other packages? If not, leave them out of common/shlibs.
  • One commit per package, one package per commit. Split each of these into a separate commit in the same PR.
  • Remove version restrictions in hostmakedepends and makedepends. Version enforcement makes no sense here because the repository specifies a single version of the package; whatever is current. It either builds with that version or it's broken.
  • What is the purpose of the Orthanc-{DicomWeb,PostgreSQL,Python} packages? They aren't reference by any other templates.
    • If Orthanc-Python really doesn't work with py3.11, that's a problem, because we'll be moving to py3.11 soon and I'd like to minimize potential breakage caused by the addition of new specialty packages in the meantime.
    • If Orthanc-Python is a Python package, it should depend on python3.

srcpkgs/Orthanc/files/orthanc/run Outdated Show resolved Hide resolved
srcpkgs/Orthanc/files/orthanc/run Outdated Show resolved Hide resolved
srcpkgs/Orthanc/files/orthanc/run Outdated Show resolved Hide resolved
common/shlibs Outdated Show resolved Hide resolved
common/shlibs Outdated Show resolved Hide resolved
srcpkgs/Orthanc-DicomWeb/template Outdated Show resolved Hide resolved
srcpkgs/Orthanc/files/Configuration.json Outdated Show resolved Hide resolved
srcpkgs/dcmtk/template Outdated Show resolved Hide resolved
@gcarlos64 gcarlos64 force-pushed the orthanc branch 3 times, most recently from f948af1 to 19da376 Compare August 8, 2022 19:25
@gcarlos64
Copy link
Author

@ahesford

* Are all of these shared libs really necessary for linking by other packages? If not, leave them out of `common/shlibs`.

Done.

* One commit per package, one package per commit. Split each of these into a separate commit in the same PR.

Done.

* Remove version restrictions in `hostmakedepends` and `makedepends`. Version enforcement makes no sense here because the repository specifies a single version of the package; whatever is current. It either builds with that version or it's broken.

Done.

* What is the purpose of the `Orthanc-{DicomWeb,PostgreSQL,Python}` packages? They aren't reference by any other templates.

These are some plugins i decided to package, it isn't requiered by the main package (Orthanc), but as these makedepends of Orthanc I added them into this PR.

  * If `Orthanc-Python` really doesn't work with py3.11, that's a problem, because we'll be moving to py3.11 soon and I'd like to minimize potential breakage caused by the addition of new specialty packages in the meantime.

It works, you just need to pass a flag with the major python version. As I passed python 3.10, i decided to restrict it on the template, but I understand now that it doesn't make sense and I've removed it. Now I need to get the python version from the upstream repo and pass it to configure_args. I'll try some way to do it.

  * If `Orthanc-Python` is a Python package, it should depend on `python3`.

It isn't, it's mostly like a binding instead.

@Chocimier
Copy link
Member

I need to get the python version from the upstream repo and pass it to configure_args

see https://github.com/void-linux/void-packages/blob/master/Manual.md#python-packages

@paper42 paper42 added the new-package This PR adds a new package label Aug 8, 2022
@gcarlos64
Copy link
Author

I need to get the python version from the upstream repo and pass it to configure_args

see https://github.com/void-linux/void-packages/blob/master/Manual.md#python-packages

thx, now I'm using the py3_ver var.

Copy link
Member

@Chocimier Chocimier left a comment

Choose a reason for hiding this comment

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

Please check that it still builds after a round of review.

srcpkgs/Orthanc-DicomWeb/template Outdated Show resolved Hide resolved
srcpkgs/Orthanc/template Outdated Show resolved Hide resolved
srcpkgs/dcmtk/template Outdated Show resolved Hide resolved
@gcarlos64
Copy link
Author

Should I specify python3 on depends of Orthanc-Python? It don't explicity depends, but if it's compiled with python 3.10, as example, you cannot use it with python 3.11, as you need to compile with the specific python version flag. I'm thinking in put something like depends="python3>=${py3_ver}<$(( ${py3_ver} + 1 ))" (probably it shell expansion don't work, it's just to illustrate my idea)

@ahesford
Copy link
Member

ahesford commented Aug 9, 2022

No, just depend on python3. We only support one version of py3 in the repository and I bump every Python package when we do a minor version jump.

@gcarlos64
Copy link
Author

No, just depend on python3. We only support one version of py3 in the repository and I bump every Python package when we do a minor version jump.

Of course, but I'm thinking on some case as version hold by user. In that example, the xbps shouldn't allow to upgrade python to 3.11 if the user holds Orthanc-Python on a version wich depends on 3.10.

@gcarlos64
Copy link
Author

Now all the packages are building successfully on my local.

I'll try to cross build to the others archs now.

@Chocimier
Copy link
Member

version hold by user

This is already covered by dynamic linking tracking, look for SONAME: libpython3.10.so.1.0 <-> python3>=3.10.0_1 in build log.

@gcarlos64
Copy link
Author

gcarlos64 commented Aug 9, 2022

version hold by user

This is already covered by dynamic linking tracking, look for SONAME: libpython3.10.so.1.0 <-> python3>=3.10.0_1 in build log.

Ahh okay, so don't I need to specify python3 on depends?

@gcarlos64 gcarlos64 force-pushed the orthanc branch 4 times, most recently from 9678dfe to 409ea61 Compare August 10, 2022 02:31
@gcarlos64
Copy link
Author

I marked all packages (except civetweb) since dcmtk, a dependency of Orthanc, need running some code on target arch to generate a header for crossbuild. It can be done running a compiled test (a single .cc present on distfile) on each arch and providing its generated headers together to package and copying it to desired directory, what seems a little tricky to maintain.

@gcarlos64 gcarlos64 changed the title New package: Orthanc-1.11.1 New package: Orthanc-1.11.2 Nov 17, 2022
@gcarlos64
Copy link
Author

I just updated Orthanc and Orthanc-DicomWeb to upstream version and made a change o Orthanc template accordingly Orthanc documentation. Also I tested the build on native aarch64 and everything seems fine.

@github-actions
Copy link

Pull Requests become stale 90 days after last activity and are closed 14 days after that. If this pull request is still relevant bump it or assign it.

@github-actions github-actions bot added the Stale label May 10, 2023
@github-actions github-actions bot closed this May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-package This PR adds a new package Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants