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

x11-terms/cool-retro-term: Bump cool-retro-term to 1.2.0-r1 and added 9999 ebuild #24553

Closed
wants to merge 1 commit into from

Conversation

thamognya
Copy link
Contributor

@thamognya thamognya commented Mar 14, 2022

Changes:

  • added 1.2.0 version ebuild
  • added 9999 version ebuild
  • Added Manifest updates
  • EAPI to 8
  • Requesting to be a proxy-maint or proxy-maintainer or maintainer for this package x11-terms/cool-retro-term

Signed-off-by: Thamognya Kodi contact@thamognya.com

@gentoo-bot
Copy link

Pull Request assignment

Submitter: @ThamognyaKodi
Areas affected: ebuilds
Packages affected: x11-terms/cool-retro-term

x11-terms/cool-retro-term: @gentoo/proxy-maint (maintainer needed)

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.


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 maintainer-needed There is at least one affected package with no maintainer. Review it if you can. assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits. labels Mar 14, 2022
Copy link
Member

@juippis juippis left a comment

Choose a reason for hiding this comment

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

Also please fix these issues in your git branch, squash the fixes to your original commit and force push the branch. No need to keep closing PRs and opening new ones, we lose this review history.

Comment on lines 48 to 49
emake
mv "${WORKDIR}/${P}/${PN}" "/usr/bin/${PN}"
Copy link
Member

Choose a reason for hiding this comment

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

You'll most likely want to replace this with something like:

	emake INSTALL_ROOT="${D}" install
	einstalldocs

"/usr/bin/${PN}" causes a sandbox error, you'll have to prepend it with "${ED}/usr/bin/${PN}"

It's good to take a look at how other packages using this eclass handle building software.
https://qa-reports.gentoo.org/output/eclass-usage/qmake-utils.txt

}

pkg_postrm() {
rm -rf "/usr/bin/${PN}" || die
Copy link
Member

Choose a reason for hiding this comment

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

This looks rather scary too.

LICENSE="GPL-2 GPL-3"
SLOT="0"
KEYWORDS="~amd64 ~ppc64 ~x86"
IUSE=""
Copy link
Member

Choose a reason for hiding this comment

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

This empty variable can be removed.

Comment on lines 4 to 7
<maintainer type="person">
<email>contact@thamognya.com</email>
<name>Thamognya Kodi</name>
</maintainer>
Copy link
Member

Choose a reason for hiding this comment

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

You know you don't have to become a maintainer to contribute for a package? Happy to merge drive-bys!

@thamognya
Copy link
Contributor Author

Ah I see the errors now, I have tried to follow the suggestions made. Thank you for the feedback as I am starting out with writing gentoo ebuilds.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2022-03-18 14:42 UTC
Newest commit scanned: 3cc0f88
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/c28af4db5e/output.html

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2022-03-19 04:17 UTC
Newest commit scanned: 6457846
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/c53c9aae03/output.html

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2022-03-19 04:32 UTC
Newest commit scanned: ba526df
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/345dd31780/output.html

@thesamesam
Copy link
Member

Looking good! Could you squash the fixup commits, then force push?

Ebuild for 1.2.0-r2 version.

Signed-off-by: Thamognya Kodi <contact@thamognya.com>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2022-03-20 04:47 UTC
Newest commit scanned: 73f9522
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/9e15b36b74/output.html

Copy link
Member

@juippis juippis left a comment

Choose a reason for hiding this comment

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

I don't know if it's me or this package, but I can't launch it.

# cool-retro-term 
QQmlApplicationEngine failed to load component
qrc:/main.qml:143:5: Type TerminalContainer unavailable
qrc:/TerminalContainer.qml:47:5: Type PreprocessedTerminal unavailable
qrc:/PreprocessedTerminal.qml:123:9: Cannot assign to non-existent property "blinkingCursor"
Cannot load QML interface

And this seems to be a rather common unsolved error,
Swordfish90/cool-retro-term#592
Swordfish90/cool-retro-term#700
Swordfish90/cool-retro-term#682
Swordfish90/cool-retro-term#592
Swordfish90/cool-retro-term#620
NixOS/nixpkgs#12027

so I really don't know what to do here. All I can say is that 1.1.1-r2 works for me... so it's looking bad. Also you shouldn't revbump 1.2.0 to -r2 in this PR since it hasn't been introduced to the tree. 1.2.0 is fine.


src_install() {
emake
mv "${WORKDIR}/${P}/${PN}" "/usr/bin/${PN}"
Copy link
Member

Choose a reason for hiding this comment

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

(FYI these issues are present in the -9999 still)
Do we want a -9999 ebuild for this?

@thamognya
Copy link
Contributor Author

I have been looking into the same problem on my side to. The issue is something I can not really find, but my original and 9999 ebuild I submitted with the non-devmanual / risky way of code fixed it, but as you said in your previous comments, they look a bit risky and even I as a end user would probably not use it. I am thinking of making a patch or such, because the issue lies in the Makefile (or there might be a extra step mentioned in one of the issues) of this version (at least from my research). I think I will try to fix on that or make this pr a draft and submit a working version soon.

@juippis
Copy link
Member

juippis commented Mar 21, 2022

The problem with the previous ebuild was that there's no way you could install it using portage due to sandbox protection.

But if you've detected the root cause for this issue and can make a patch, that sounds very promising! It could also help upstream.

@thamognya thamognya marked this pull request as draft April 24, 2022 01:59
@techflashYT
Copy link

Any update on this? Would love to have 1.2.0 on gentoo

beatussum added a commit to beatussum/gentoo that referenced this pull request Jul 20, 2023
Closes: gentoo#24553
Closes: https://bugs.gentoo.org/880661
Signed-off-by: Mattéo Rossillol‑‑Laruelle <beatussum@protonmail.com>
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). maintainer-needed There is at least one affected package with no maintainer. Review it if you can. no bug found No Bug/Closes found in the commits.
Projects
None yet
6 participants