-
-
Notifications
You must be signed in to change notification settings - Fork 3.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
addpkg(x11/meld): 3.22.2 #22744
base: master
Are you sure you want to change the base?
addpkg(x11/meld): 3.22.2 #22744
Conversation
x11-packages/meld/build.sh
Outdated
termux_setup_glib_cross_pkg_config_wrapper | ||
} | ||
|
||
# It is really strange that this is necessary. The reason that meson is installing this |
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 comment is not required here.
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.
Do you know what the root cause of the problem might be? Why do no other Termux packages or other distros' packages I could find need to do this moving of the folder? It is not obvious to me what is wrong which makes it seem like I made a mistake somehow, requiring this messy solution.
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.
After searching for a very long time, I found the root cause of this problem.
I wrote a huge text wall but it was way too huge, so to skim over the explanation really quickly to it's more clear, that incorrect path happens because of not building in a Python Venv when building from a docker container, to build in a Python Venv from the docker container, TERMUX_PKG_SETUP_PYTHON=true
must be set, so I set that, but also, setting that causes itstool
to produce errors when building in the docker container but not on device, so I then have to copy the itstool
TERMUX_PKG_SETUP_PYTHON=true
fix code from the orca
package, also I then have to migrate orca
's autotools itstool
fix to be compatible with Meson since the orca
package is older,
so in reality, the "other termux package with a similar workaround" I was looking for was really the orca
package, it was just hard to find that because the correct workaround code looks so different from the sloppy workaround code I made separately, and I had to figure that out.
cf43de4
to
5755557
Compare
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.
LGTM.
x11-packages/meld/build.sh
Outdated
|
||
termux_step_configure_meson() { | ||
termux_setup_meson | ||
# this is an addition that will probably also have to be made to the orca package |
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 comment is not related to this package.
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.
but a short, oneline comment should be added why this condition is added.
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.
Thanks, it is important for comments to be useful for others rather than just the author, and I struggle with writing comments that are clear to others. I tried to improve the comments, and now how do they look to you?
Closes termux#18900 (that bug was described as "with the general intention of running meld", and this is a newer way and a newer version of meld than that)
A GUI folder diff tool for X11. Packaged in many distros
Closes #18900
(that bug was described as "with the general intention of running meld", and this is a newer way and a newer version of meld than that)