https://bugzilla.redhat.com/show_bug.cgi?id=1758626 Ankur Sinha (FranciscoD) <sanjay.ankur@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED --- Comment #5 from Ankur Sinha (FranciscoD) <sanjay.ankur@xxxxxxxxx> --- (In reply to Qianqian Fang from comment #4) > @Ankur, thanks for the review. see my replies below. > > > Here, the spec is building 4 different tools? > > No. the spec is for building iso2mesh only - the tools you saw are used > internally by iso2mesh and are not meant for shared by other packages (or > directly called by users). OK, so they're "private". That's fine then. Could we add comments to the spec to say so please, just for the benefit of other package maintainers who would probably end up asking the same questions as me otherwise? :) > > several of these utilities were modified/customized version specifically for > iso2mesh (such as the CGAL tools and cork); other utilities were included in > iso2mesh because the meshing result is sensitive to these utilities > versions, if I let user to call system-installed tools, such as tetgen, the > meshing output will not be reproducible across computers. OK, but we can specify versioned dependencies, would that work? Of course, it depends on whether or not the Fedora packages are carrying the version you need. Fedora generally carries the latest versions, though---and that in a way helps us remind developers to update their code bases too: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_package_dependencies https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/ Bundling is no longer forbidden in Fedora, but if at all possible, it should be avoided: https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling > > I also want to mention that these tools are only used by about 5-10% of the > functions in iso2mesh. They are dependencies, but weak dependencies. Sure, they can be specified as weak deps if needed also: https://docs.fedoraproject.org/en-US/packaging-guidelines/WeakDependencies/ > > > > tetgen is already in Fedora, by the way, so it must be used as a BuildRequires or Requires as required > > I can remove tetgen from iso2mesh package and add a link in its place to use > the system installed tetgen, but the risk is that iso2mesh output will not > be deterministic. Is there a way of checking this maybe? Is there a testsuite, for example? The version of tetgen in Fedora is 1.5.x. > > > if the others are dependencies, they must be packaged separately. (I know this means more reviews, but the point is that each tool must be installable on its own also.) > > again, my purpose of including these utilities inside iso2mesh is to ensure > that the meshing output is reproducible (many utilities generate very > different output from version to version, such as CGAL and tetgen); also > such inclusion was permitted under their respective open-source licenses. Sure, I understand that, but please see comments I've made before. > > It is fine if someone want to package these tools as separate packages, but > 1) the tools that iso2mesh used are either modified, or older versions, 2) I > am not the upstream author of these utilities , for example, I am not the > author of cork (upstream repo: https://github.com/gilbo/cork), iso2mesh uses > a modified version in my fork (https://github.com/fangq/cork). So, if we > package my fork as the official package, I am not sure how to accommodate > the upstream development in the future. I've looked at cork---upstream is inactive, has been for a while, and from the README, it really does not look like they'll actively maintain the project in the future either. I've built it using your patches, and I had to tweak the Makefile to get it to generate shared libraries also---static libraries are discouraged: https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries https://ankursinha.fedorapeople.org//cork/ If that will do, I can submit it for review? > > > I do want to mention that iso2mesh currently contains a copy of jsonlab and > jnifti (used by a couple of file io functions). Because "pkg load" in octave > can't check/load dependencies, AFAIK, so I think including a copy of these > toolboxes in iso2mesh is a more portable approach, despite some redundancy. I'll have to check this up. Maybe we can speak to the Octave maintainer on the sci-tech mailing list to see what the best way of handling these are? https://lists.fedoraproject.org/admin/lists/scitech.lists.fedoraproject.org/ -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
