-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Package lib3mf #40307
Package lib3mf #40307
Conversation
Should be ready to merge, I just used the OpenSCAD build to successfully compile a decently complicated design into 3mf. |
0c13bf6
to
ecef56b
Compare
ecef56b
to
ede624e
Compare
Probably should have ran the linter... |
Just fixed the indentation and rebuilt, the only change was the indentation so it should be fine if the rest of the checks pass. |
@paper42 - Sorry for any confusion, I pushed another commit during the CI run so it does actually need to run again. This time, I've ensured that the linter passes and the project compiles, so CI should pass this time. |
srcpkgs/lib3mf/template
Outdated
configure_args="-DUSE_INCLUDED_LIBZIP=OFF -DUSE_INCLUDED_ZLIB=OFF | ||
-DUSE_INCLUDED_GTEST=OFF -DUSE_INCLUDED_SSL=OFF | ||
-DCMAKE_INSTALL_INCLUDEDIR='include/$pkgname' -DCMAKE_INSTALL_PREFIX=/usr | ||
$([ "$XBPS_CHECK_PKGS" ] || echo '-DLIB3MF_TESTS=OFF')" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this would be better as an if block below:
if [ -z "$XBPS_CHECK_PKGS" ]; then
configure_args+=" -DLIB3MF_TESTS=OFF"
fi
however, I don't think it will affect the final build if the tests are always built
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests have dependencies, so they won't compile correctly by default AFAIK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that can be added to make_depends
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not entirely sure what the point of check dependencies are then, but if it's more idiomatic I'll merge them.
srcpkgs/openscad/template
Outdated
build_options="3mf" | ||
desc_option_png="Enable support for 3MF models" | ||
build_options_default="3mf" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why make this a build option?
also, these variables should go before the pre_patch()
function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not having it is the behavior beforehand and the library is optional, so I figured it qualified as an optional feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the positioning
ede624e
to
6b6d69a
Compare
6b6d69a
to
3af85cd
Compare
Alright, I had to move the symlinking to |
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. |
Testing the changes
New package