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/cura (legacy versions) #1500

Closed
wants to merge 3 commits into from

Conversation

tomboy-64
Copy link
Contributor

Let's try this again, shall we?

@axs-gentoo
@Amynka
@gentoo/proxy-maint

I had to open a new pull-request due to rebases and forced pushes. Improvement suggestions from PRs 1377 and 1407 have been incorporated as I saw fit.


make_desktop_entry cura \
Cura \
/usr/share/pixmaps/cura.png \
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be changed to ${PN} as you call newicon() above and because make_desktop_entry() doesn't prepend EPREFIX

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This implements your suggestion from #1377. Reverting to the original.

Copy link
Contributor

Choose a reason for hiding this comment

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

This implements your suggestion from #1377.

Nope, it does not.

My suggestion was:

It'd be much cleaner to do newicon ... ${PN}.png+${PN}.png

Your original ebuild didn't have newicon at all. In this ebuild you've just mixed all up. Either remove newicon and prepend EPREFIX or leave newicon and change this line to ${PN}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. Looking over it again, I think I have a better grasp of it now. Hope you'd agree?

@monsieurp monsieurp added version bump assigned PR successfully assigned to the package maintainer(s). labels May 25, 2016
@tomboy-64 tomboy-64 force-pushed the cura-legacy branch 2 times, most recently from e026a18 to a66a16d Compare June 5, 2016 00:21
@tomboy-64
Copy link
Contributor Author

tomboy-64 commented Jun 5, 2016

Re-added the notice "based on the original ebuild by AxS" in the commit message, which got lost somewhere along the way.

IUSE=""

RDEPEND="${PYTHON_DEPS}
dev-python/wxpython:3.0[${PYTHON_USEDEP}]
Copy link
Contributor

Choose a reason for hiding this comment

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

It has to have [opengl,${PYTHON_USEDEP}] otherwise it will not work

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. has been updated.

echo ${PV} > "${ED}"usr/share/cura/version || die
cat > "${T}"/cura <<- CURAEOF || die
#!/bin/sh
PYTHONPATH="\$PYTHONPATH:${EROOT}usr/share/cura/" "${PYTHON}" "${EROOT}usr/share/cura/cura.py" "\$@"
Copy link
Member

Choose a reason for hiding this comment

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

referencing ROOT in src_* phases is invalid [0]. Straight '/' should suffice here. Optionally, you can move this to pkg_preinst and do it there.

[0] https://devmanual.gentoo.org/ebuild-writing/variables/index.html#root

@gktrk gktrk removed the assigned PR successfully assigned to the package maintainer(s). label Jun 22, 2016
@gktrk
Copy link
Member

gktrk commented Jun 22, 2016

@gentoo/proxy-maint

@Amynka
Copy link
Contributor

Amynka commented Jun 22, 2016

So where are we here can you fix those stuff which gokturk told you?

Thanks

@tomboy-64
Copy link
Contributor Author

I will see it's getting done this weekend.

Marshall Brewer (Gentoo Key) added 3 commits July 4, 2016 16:26
based on the original ebuild by _AxS_

Package-Manager: portage-2.2.28
based on the original ebuild by _AxS_

Package-Manager: portage-2.2.28
based on the original ebuild by _AxS_

Package-Manager: portage-2.2.28
@tomboy-64
Copy link
Contributor Author

Thanks for the suggestions. I applied the changes and did brief runtests. Looks good to me now.

src_prepare() {
cat > "${T}"/cura <<- CURAEOF || die
#!/bin/sh
PYTHONPATH="\$PYTHONPATH:${EROOT}usr/share/cura/" "${PYTHON}" "${EROOT}usr/share/cura/cura.py" "\$@"
Copy link
Member

Choose a reason for hiding this comment

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

Referencing ROOT in src_* phases is forbidden. See: https://devmanual.gentoo.org/ebuild-writing/variables/index.html#root

Unless you want to support prefix, you can safely assume that ROOT is always /.

Copy link
Member

Choose a reason for hiding this comment

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

I think s|${EROOT}|${EPREFIX}/|g is safe here

@monsieurp
Copy link
Contributor

ping

@monsieurp
Copy link
Contributor

I said ping.


make_desktop_entry cura \
Cura \
"${EROOT}/usr/share/pixmaps/cura.png" \
Copy link
Member

Choose a reason for hiding this comment

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

the / after ${EROOT} is not needed as the variable is guaranteed to end in a trailing slash.

@gktrk gktrk self-assigned this Aug 8, 2016
@gktrk
Copy link
Member

gktrk commented Aug 8, 2016

Merged in 0a0d8e8...926306e. Thanks!

@gktrk gktrk closed this Aug 8, 2016
@tomboy-64 tomboy-64 deleted the cura-legacy branch April 23, 2017 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants