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

media-gfx/nomacs: add 3.17.2285 #32397

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

listout
Copy link
Contributor

@listout listout commented Aug 20, 2023

@gentoo-bot
Copy link

Pull Request assignment

Submitter: @listout
Areas affected: ebuilds
Packages affected: media-gfx/nomacs

media-gfx/nomacs: @gentoo/qt

Linked bugs

No bugs to link found. If your pull request references any of the Gentoo bug reports, please add appropriate GLEP 66 tags to the commit message and request reassignment.

If you do not receive any reply to this pull request, please open or link a bug to attract the attention of maintainers.


In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits. labels Aug 20, 2023
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2023-08-20 19:10 UTC
Newest commit scanned: ecef991
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/023e96cd9d/output.html

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2023-08-20 19:55 UTC
Newest commit scanned: 92c894a
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/3a1c742224/output.html

Copy link
Member

@a17r a17r left a comment

Choose a reason for hiding this comment

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

closes: https://bugs.gentoo.org/906488
^ apparently this does not work case-insensitive, that's why the PR did not get linked in bug

};

+#if EXIV2_TEST_VERSION(0, 28, 0)
+ Exiv2::Image::UniquePtr mExifImg; // TODO std::unique_ptr<Exiv2::Image> (and all other *::AutoPtr)
Copy link
Member

Choose a reason for hiding this comment

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

// TODO std::unique_ptr<Exiv2::Image> (and all other *::AutoPtr)

See also:
openstreetmap/merkaartor@1e20d2c

Using that would avoid #ifdefs and maybe result in an upstream-presentable PR for them to consider?

Alternatively, use auto: https://invent.kde.org/graphics/kphotoalbum/-/commit/82520a00bb8ca3f6400c7cc053066072e5055594

DOCS=( src/changelog.txt )

PATCHES=(
"${FILESDIR}"/${P}-exiv-0.28.0-build-fix.patch
Copy link
Member

Choose a reason for hiding this comment

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

add # bug 906488 ref

Comment on lines 59 to 61
# Fix bug #847112
sed -e 's:QT_QMAKE_EXECUTABLE NAMES "qmake" "qmake-qt5" "qmake.exe":QT_QMAKE_EXECUTABLE NAMES "qmake" "qmake5" "qmake.exe":' \
-i plugins/cmake/Utils.cmake || die
Copy link
Member

Choose a reason for hiding this comment

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

Please drop that ugly sed and simply set -DQT_QMAKE_EXECUTABLE=qmake5 in mycmakeargs instead. See also https://bugs.gentoo.org/847112

Comment on lines 63 to 64
sed -e "s:lib/nomacs-plugins:$(get_libdir)/nomacs-plugins:" \
-i plugins/cmake/Utils.cmake || die
Copy link
Member

@a17r a17r Sep 3, 2023

Choose a reason for hiding this comment

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

This can be replaced with upstream commit: nomacs/nomacs-plugins@e1d32cd

Of course not without adjusting paths.

Actually, that sed is obsolete with your version bump. And that's yet another example why sed is bad.

Comment on lines 65 to 66
sed -e "s:lib/nomacs-plugins:$(get_libdir)/nomacs-plugins:" \
-i src/DkCore/DkPluginManager.cpp || die
Copy link
Member

@a17r a17r Sep 3, 2023

Choose a reason for hiding this comment

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

This could be replaced with upstream commit: nomacs/nomacs@c8f6c32

Of course not without adjusting paths and dropping unrelated patch hunks (version bump, subproject)

Actually, that sed is obsolete with your version bump. And that's yet another example why sed is bad.

DESCRIPTION="Qt-based image viewer"
HOMEPAGE="https://nomacs.org/"
SRC_URI="https://github.com/${PN}/${PN}/archive/${PV}.tar.gz -> ${P}.tar.gz
plugins? ( https://github.com/${PN}/${PN}-plugins/archive/$(ver_cut 1-2).tar.gz -> ${PLUGIN_PKG}.tar.gz )"
Copy link
Member

Choose a reason for hiding this comment

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

How is that supposed to work without nomacs-plugins-3.17 having been tagged? What exactly did you manifest there?

>>> Downloading 'https://github.com/nomacs/nomacs-plugins/archive/3.17.tar.gz'
--2023-09-03 11:51:29--  https://github.com/nomacs/nomacs-plugins/archive/3.17.tar.gz
Resolving github.com (github.com)... 140.82.121.3
Connecting to github.com (github.com)|140.82.121.3|:443... connected.
HTTP request sent, awaiting response... 302 Found
Location: https://codeload.github.com/nomacs/nomacs-plugins/tar.gz/3.17 [following]
--2023-09-03 11:51:29--  https://codeload.github.com/nomacs/nomacs-plugins/tar.gz/3.17
Resolving codeload.github.com (codeload.github.com)... 140.82.121.10
Connecting to codeload.github.com (codeload.github.com)|140.82.121.10|:443... connected.
HTTP request sent, awaiting response... 404 Not Found
2023-09-03 11:51:30 ERROR 404: Not Found.

!!! Couldn't download 'nomacs-plugins-3.17.tar.gz'. Aborting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I couldn't find it/its not tagged

@listout listout marked this pull request as draft September 3, 2023 10:04
@listout listout force-pushed the nomacs-exiv-0.28-patch branch 2 times, most recently from 1f67f69 to 256db5d Compare September 3, 2023 12:37
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2023-09-03 12:55 UTC
Newest commit scanned: 256db5d
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/4d7e2e6fe4/output.html

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2023-09-03 14:10 UTC
Newest commit scanned: 0ae1234
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/c25b8605b8/output.html

@listout
Copy link
Contributor Author

listout commented Sep 3, 2023

I've made a PR upstream, waiting for replies from dev.

nomacs/nomacs#982

cc: @a17r

@novomesk
Copy link
Contributor

Try to use following instead:

https://github.com/nomacs/nomacs/releases/tag/3.17.2285

https://github.com/novomesk/nomacs-plugins/releases/tag/3.17.2285

@listout listout marked this pull request as ready for review September 15, 2023 18:16
@listout listout changed the title media-gfx/nomacs: add 3.17.2282, and fix build with exiv2 0.28.0 media-gfx/nomacs: add 3.17.2285 Sep 15, 2023
Closes: https://bugs.gentoo.org/906488
Signed-off-by: Brahmajit Das <brahmajit.xyz@gmail.com>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2023-09-15 18:41 UTC
Newest commit scanned: 16a1c14
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/1102fd000b/output.html

@novomesk
Copy link
Contributor

@a17r Andreas, what is your opinion about the upgraded version?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits.
Projects
None yet
5 participants