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

[salome-med-fichier] new port #30893

Merged
merged 9 commits into from Jul 21, 2023
Merged

Conversation

Neumann-A
Copy link
Contributor

@Neumann-A Neumann-A commented Apr 15, 2023

  • Changes comply with the maintainer guide
  • The name of the port matches an existing name for this component on https://repology.org/ if possible, and/or is strongly associated with that component on search engines.
  • Optional dependencies are resolved in exactly one way. For example, if the component is built with CMake, all find_package calls are REQUIRED, are satisfied by vcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx
  • The versioning scheme in vcpkg.json matches what upstream says.
  • The license declaration in vcpkg.json matches what upstream says.
  • The installed as the "copyright" file matches what upstream says.
  • [?] The source code of the component installed comes from an authoritative source. (There is no authoritative source; getting it form salome is what other packagers do and probably the best we could do)
  • The generated "usage text" is accurate. See adding-usage for context.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is in the new port's versions file.
  • Only one version is added to each modified port's versions file.

removes tag from #13752

@Cheney-W Cheney-W added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Apr 17, 2023
@Cheney-W
Copy link
Contributor

Usage test passed with x64-windows triplet:

    find_package(MEDFile CONFIG REQUIRED)
    target_link_libraries(main PRIVATE medC)

@Cheney-W Cheney-W added the info:reviewed Pull Request changes follow basic guidelines label Apr 17, 2023
@@ -0,0 +1,81 @@
# This library cannot easily be found only. Be aware that the original source repository is not accessible.
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. That is something different which even depends on med-fichier via salome-kernel. I also have a port for salome-med which i'll add later if i got the other deps for it in. There is also somewhere a webpage somewhere explaining the difference between all those med named stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BillyONeal BillyONeal added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. and removed info:reviewed Pull Request changes follow basic guidelines labels Apr 18, 2023
@BillyONeal
Copy link
Member

Tagging requires: vcpkg-team-review for 2nd opinion on where the sources are coming from. I'm weakly in favor of merging this as is.

@Neumann-A
Copy link
Contributor Author

Tagging requires: vcpkg-team-review for 2nd opinion on where the sources are coming from.

A few links to read:
https://dev.opencascade.org/content/sorry-offtopic-researching-development-edfs-med-fichier
https://forum.freecad.org/viewtopic.php?style=3&p=257912

@BillyONeal
Copy link
Member

A few links to read: https://dev.opencascade.org/content/sorry-offtopic-researching-development-edfs-med-fichier https://forum.freecad.org/viewtopic.php?style=3&p=257912

Right, the question is whether we are willing to include such a library in the curated catalog when it seems to have no accountable upstream in the first place.

@Neumann-A
Copy link
Contributor Author

Right, the question is whether we are willing to include such a library in the curated catalog when it seems to have no accountable upstream in the first place.

If it is good enough to be packaged by major linux distros (also as libmedc; i choose the name med-fichier since that seems to be easier to google) isn't it good enough to be packaged by vcpkg?
Without it you would completely exclude all of https://www.salome-platform.org/.

@BillyONeal
Copy link
Member

If it is good enough to be packaged by major linux distros (also as libmedc; i choose the name med-fichier since that seems to be easier to google) isn't it good enough to be packaged by vcpkg?

https://repology.org/project/libmedc/versions Only shows Alpine. When I Googled all I found was that entry for the Debian package. When I try to look at salome-platform.org they seem to be one of those 'tell us all of your information and then we'll give you sources' outfits like cudnn, which we similarly don't index.

However, as part of writing this reply I tried messing with repology's URI, and https://repology.org/project/med-fichier/versions redirects to https://repology.org/project/med/versions . But that naming suggests https://git.salome-platform.org/gitweb/?p=modules/med.git and you said this is something else. The fedora package redirects to https://www.salome-platform.org/user-section/about/med which is a 404.

I'm just saying this is confusing enough that I want a 2nd opinion, not that I personally would argue against merging this.

@Neumann-A
Copy link
Contributor Author

But that naming suggests https://git.salome-platform.org/gitweb/?p=modules/med.git and you said this is something else.

This is the salome "module" which is probably ParaMEDMEM from the who is who link (at least if I look at the file tree it looks like that ).

The fedora package redirects

Better look at the actual build recipes of the different distros where they download from https://files.salome-platform.org/Salome/other/med-${VERSION}.tar.gz . (Note that this is in the subfolder "other" which turns out to be med-fichier)

@BillyONeal BillyONeal removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Apr 21, 2023
@BillyONeal
Copy link
Member

@ras0219-msft @vicroms @dan-shaw @JavierMatosD @markle11m and I discussed this today.

Open questions:

  • We do not want to merge this as is because we can't apply policies to it / contact upstream, etc. If this is an 'essential core component' that the whole world depends on that would be one thing, like OpenSSL, we might have to make accommodations, but that doesn't seem to be the case here. If we can find somewhere where upstream is responsive this may change. It's possible that upstream is responsive, but we don't know where that is, and the 'solution' would be finding that place.
  • How are people in the wild actually consuming this thing? You showed two links where people are confused as to where the source is, but clearly they have official binary downloads and similar... do we have end users who are actually looking for any of this we can talk to?
  • How do the other salome-platform.org entities supposed to get this today (outside of vcpkg)? Is it vendored inside them or something?
  • We might want to consider using the Debian mirror of the sources instead of the 'upstream' in this case because the Debian folks actually seem to be maintaining this thing now that upstream is a 404.

If it is good enough to be packaged by major linux distros (also as libmedc; i choose the name med-fichier since that seems to be easier to google) isn't it good enough to be packaged by vcpkg?

We think this is a good question but aren't prepared to make a policy statement yet. (It is a good point)

@BillyONeal BillyONeal added the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Apr 21, 2023
@Neumann-A
Copy link
Contributor Author

How do the other salome-platform.org entities supposed to get this today (outside of vcpkg)? Is it vendored inside them or something?

med-fichier is internally hosted by EDF R&D. salome platform is used by EDF R&D and partly maintained by it.
Also it is probably not gone it is just not accessible from outside:
Mentioned http://med.der.edf.fr/ is in fact an internal Intranet site within EDF.
So my best guess on who to contact as upstream would be via https://discourse.salome-platform.org/

Debian mirror

if i member correctly the debian mirror is a few minor versions behind the salome version and does not have the CMake build system. I would rather use the salome version since it seems to be nearer to the real source even if it adds the cmake build.

@BillyONeal BillyONeal removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Jul 20, 2023
@BillyONeal
Copy link
Member

@JavierMatosD @dan-shaw @ras0219-msft @vicroms and I talked about this today. None of us like the name but we don't think anyone else is going to stomp on it. In the interest of containing potential confusion, would you accept salome-med-fichier?

@BillyONeal BillyONeal marked this pull request as draft July 20, 2023 21:13
@Neumann-A Neumann-A changed the title [med-fichier] new port [salome-med-fichier] new port Jul 20, 2023
@Neumann-A Neumann-A marked this pull request as ready for review July 20, 2023 21:22
@BillyONeal BillyONeal merged commit 50a4aa2 into microsoft:master Jul 21, 2023
15 checks passed
@BillyONeal
Copy link
Member

Thanks!

@Cheney-W Cheney-W added the info:reviewed Pull Request changes follow basic guidelines label Jul 21, 2023
@Neumann-A Neumann-A deleted the add_med-fichier branch July 21, 2023 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants