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

ktx-tools: init at 4.3.2 #267009

Merged
merged 2 commits into from
Apr 14, 2024
Merged

ktx-tools: init at 4.3.2 #267009

merged 2 commits into from
Apr 14, 2024

Conversation

bonsairobo
Copy link
Contributor

Description of changes

KTX is a standard GPU texture container format. This package includes libktx and several CLI tools for creating KTX files.

Home Page: https://github.com/KhronosGroup/KTX-Software

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

maintainers/maintainer-list.nix Outdated Show resolved Hide resolved
pkgs/by-name/kt/ktx-tools/package.nix Outdated Show resolved Hide resolved
@Lurkki14
Copy link
Member

The commit message for adding yourself to maintainers should be maintainers: add bonsairobo

pkgs/by-name/kt/ktx-tools/package.nix Outdated Show resolved Hide resolved
@Lurkki14
Copy link
Member

You need to squash commit use "maintainers" attribute already in scope into ktx-tools: init at 4.2.1

@bonsairobo
Copy link
Contributor Author

Thanks for the review @Lurkki14

pkgs/by-name/kt/ktx-tools/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/kt/ktx-tools/package.nix Outdated Show resolved Hide resolved
@AndersonTorres
Copy link
Member

Closing as dead.

@bonsairobo
Copy link
Contributor Author

@AndersonTorres Hello? I replied to your feedback months ago (twice!) and you never responded.

@AndersonTorres
Copy link
Member

I am cleaning up my backlog of 60+ notifications - I was away from my notebook in Christmas -, and noticed no movement on some PRs, many of them before the pkgs/by-name advent.

Apologies.

Now, feel free to fix the merge conflicts.

@AndersonTorres
Copy link
Member

Approving because I don't want to block this.

@bonsairobo
Copy link
Contributor Author

@AndersonTorres OK no problem. Do you see the question I asked about which formatter to use?

@AndersonTorres
Copy link
Member

Yes.
nixfmt-rfc-style is the blessed formatter.

NixOS/rfcs#166

@Aleksanaa
Copy link
Member

Hi, this package doesn't build on Linux. You can find the build log in the CI at the bottom of the PR:

FAILED: tools/imageio/CMakeFiles/imageio.dir/jpg.imageio/jpginput.cc.o 
/nix/store/kvlhk0gpm2iz1asbw1xjac2ch0r8kyw9-gcc-wrapper-13.2.0/bin/g++ -DKTX_FEATURE_KTX1 -DKTX_FEATURE_KTX2 -DKTX_FEATURE_WRITE -I/build/source/tools/imageio/. -I/build/source/lib/astc-encoder/Source -I/build/source/utils -I/build/source/lib/basisu -I/build/source/lib/dfdutils -I/build/source/other_include -I/build/source/other_projects/fmt/include -O2 -g -DNDEBUG -std=gnu++17 -fvisibility=hidden -Wall -Wextra -Werror -O3 -MD -MT tools/imageio/CMakeFiles/imageio.dir/jpg.imageio/jpginput.cc.o -MF tools/imageio/CMakeFiles/imageio.dir/jpg.imageio/jpginput.cc.o.d -o tools/imageio/CMakeFiles/imageio.dir/jpg.imageio/jpginput.cc.o -c /build/source/tools/imageio/jpg.imageio/jpginput.cc
In file included from /build/source/other_projects/fmt/include/fmt/format.h:48,
                 from /build/source/tools/imageio/./imageio.h:41,
                 from /build/source/tools/imageio/jpg.imageio/jpginput.cc:29:
/build/source/other_projects/fmt/include/fmt/core.h: In instantiation of 'constexpr fmt::v9::detail::value<Context> fmt::v9::detail::make_value(T&&) [with Context = fmt::v9::basic_format_context<fmt::v9::appender, char>; T = jpgd::jpgd_status&]':
/build/source/other_projects/fmt/include/fmt/core.h:1777:29:   required from 'constexpr fmt::v9::detail::value<Context> fmt::v9::detail::make_arg(T&&) [with bool IS_PACKED = true; Context = fmt::v9::basic_format_context<fmt::v9::appender, char>; type <anonymous> = fmt::v9::detail::type::custom_type; T = jpgd::jpgd_status&; typename std::enable_if<IS_PACKED, int>::type <anonymous> = 0]'
/build/source/other_projects/fmt/include/fmt/core.h:1901:77:   required from 'constexpr fmt::v9::format_arg_store<Context, Args>::format_arg_store(T&& ...) [with T = {jpgd::jpgd_status&}; Context = fmt::v9::basic_format_context<fmt::v9::appender, char>; Args = {jpgd::jpgd_status}]'
/build/source/other_projects/fmt/include/fmt/core.h:1918:31:   required from 'constexpr fmt::v9::format_arg_store<Context, typename std::remove_cv<typename std::remove_reference<Args>::type>::type ...> fmt::v9::make_format_args(Args&& ...) [with Context = basic_format_context<appender, char>; Args = {jpgd::jpgd_status&}]'
/build/source/other_projects/fmt/include/fmt/core.h:3206:44:   required from 'std::string fmt::v9::format(format_string<T ...>, T&& ...) [with T = {jpgd::jpgd_status&}; std::string = std::__cxx11::basic_string<char>; format_string<T ...> = basic_format_string<char, jpgd::jpgd_status&>]'
/build/source/tools/imageio/jpg.imageio/jpginput.cc:205:28:   required from here
/build/source/other_projects/fmt/include/fmt/core.h:1735:15: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
 1735 |   const auto& arg = arg_mapper<Context>().map(FMT_FORWARD(val));
      |               ^~~
/build/source/other_projects/fmt/include/fmt/core.h:1735:46: note: the temporary was destroyed at the end of the full expression 'fmt::v9::detail::arg_mapper<fmt::v9::basic_format_context<fmt::v9::appender, char> >().fmt::v9::detail::arg_mapper<fmt::v9::basic_format_context<fmt::v9::appender, char> >::map<jpgd::jpgd_status&>((* & * & val))'
 1735 |   const auto& arg = arg_mapper<Context>().map(FMT_FORWARD(val));
      |                     ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors

Does the upstream have anything to say about this? Do we need NIX_CFLAGS_COMPILE = "-Wno-error=dangling-reference"?

@bonsairobo
Copy link
Contributor Author

@Aleksanaa Looks like I need to upgrade to a newer revision

KhronosGroup/KTX-Software#737

@bonsairobo
Copy link
Contributor Author

I assume this build failure is a result of the C++ toolchain being upgraded since I last built this.

bonsairobo and others added 2 commits April 13, 2024 13:30
Co-authored-by: Jussi Kuokkanen <44469719+Lurkki14@users.noreply.github.com>
Co-authored-by: Jussi Kuokkanen <44469719+Lurkki14@users.noreply.github.com>
@bonsairobo
Copy link
Contributor Author

Upgraded the package to 4.3.2, build seems to work for me. Also reformatted with nixfmt.

@bonsairobo bonsairobo changed the title ktx-tools: init at 4.2.1 ktx-tools: init at 4.3.2 Apr 13, 2024
Copy link
Member

@Aleksanaa Aleksanaa left a comment

Choose a reason for hiding this comment

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

Tested

@Aleksanaa Aleksanaa merged commit ed38987 into NixOS:master Apr 14, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants