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

Migrate from setuptools to Meson and meson-python #1071

Closed
wants to merge 8 commits into from

Conversation

chewi
Copy link
Member

@chewi chewi commented Jul 21, 2023

This makes Portage PEP 517 compliant.

When building via meson-python, the man pages and logrotate config are no longer included as there seems little point.

@chewi chewi force-pushed the meson branch 2 times, most recently from 2495cce to ee07c78 Compare July 21, 2023 15:51
Copy link
Member

@thesamesam thesamesam left a comment

Choose a reason for hiding this comment

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

Thank you so much for tackling this!

meson.build Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
.builds/ci.yml Show resolved Hide resolved
@chewi chewi force-pushed the meson branch 27 times, most recently from 85fc327 to 1343db8 Compare July 23, 2023 17:05
@@ -0,0 +1,133 @@
install_data(
[
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what you mean by this. The first thing that install_data does when receiving a "list of strings or list of files() objects" parameter is internally convert all bare strings into files() objects using the same code that files() uses to check if they exist up front.

Are you aware of cases where this does not happen? If so, then it is a meson bug, because the configure stage should never succeed when the build or install stages can be trivially detected as impossible to succeed, such as for example when rule inputs are missing.

The difference between using strings and using files() objects is supposed to be limited to the fact that files() can be defined in one subdir and used in another subdir while remembering which subdir they were defined in (and therefore always resolving relative to the subdir they were defined in rather than the subdir they were used in).

'glsa-check',
'gpkg-sign',
'portageq',
'quickpkg'
Copy link
Contributor

Choose a reason for hiding this comment

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

Although it isn't specified anywhere, muon fmt (an autoformatter provided by the c99 implementation of meson) uses four spaces by default.

Personally I've always been a fan of four spaces, for the same reason I like them in python (and also bash and C/C++).

@chewi
Copy link
Member Author

chewi commented Jul 30, 2023

My suggestion: let's get rid of 3.7 and 3.8 before we merge this, it makes the reviewing a bit simpler.

We've already done that since I opened this.

@SoapGentoo SoapGentoo requested review from eli-schwartz and removed request for eli-schwartz July 30, 2023 15:19
@chewi chewi force-pushed the meson branch 3 times, most recently from 2adbb48 to d901fa5 Compare July 30, 2023 22:39
@chewi chewi changed the title WIP Migrate from setuptools to Meson and meson-python Migrate from setuptools to Meson and meson-python Jul 30, 2023
@thesamesam
Copy link
Member

thesamesam commented Aug 1, 2023

As far as I'm concerned, we can merge this now, and pick up any pieces once it gets a bit more testing. Any objections?

I've been running it for a few days and everything seems fine, although there were 2 missing files I commented about on the ebuild side (gentoo/gentoo#32098 (comment)). I've not investigated that at all and it might've been fixed since.

@chewi
Copy link
Member Author

chewi commented Aug 1, 2023

As far as I'm concerned, we can merge this now, and pick up any pieces once it gets a bit more testing. Any objections?

I've been running it for a few days and everything seems fine, although there were 2 missing files I commented about on the ebuild side (gentoo/gentoo#32098 (comment)). I've not investigated that at all and it might've been fixed since.

Sorry, I missed that comment. I wasn't aware of that issue so I'll take a look.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@chewi
Copy link
Member Author

chewi commented Aug 1, 2023

I've been running it for a few days and everything seems fine, although there were 2 missing files I commented about on the ebuild side (gentoo/gentoo#32098 (comment)). I've not investigated that at all and it might've been fixed since.

elog-save-summary was one I simply missed. Fixed that now.

bashrc-functions.sh was always there, but the mode has changed, along with some other files in portage-bindir. They were previously all marked executable. Is that what your report was flagging?

This makes Portage PEP 517 compliant.

When building via meson-python, the man pages and logrotate config are
no longer included as there seems little point.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
Signed-off-by: James Le Cuirot <chewi@gentoo.org>
Signed-off-by: James Le Cuirot <chewi@gentoo.org>
The tests call _disable_legacy_globals, but they never reset
portage.data._initialized_globals, meaning that the globals never get
set again.

This led to the test trying to chown with GID 0 as an unprivileged user.
This only failed under CI after switching to pytest, despite it working
with runTests.py, as well as locally. Running it on its own also worked
fine. I don't know how it worked before, maybe it was just the ordering
and some luck.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
This is particularly important when Portage is installed in a venv to
ensure that other scripts are launched using the same environment.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
It's not clear what bin_entry_point.sh was trying to do before. The
regular expression didn't appear to match any likely shebang. The
wrapper already runs under the desired Python, so we only need to use
sys.executable, which points to the venv's python symlink.

We don't need to worry about handling non-Python scripts any more either
as the new Meson-based build system just installs these directly to bin
rather than creating an entrypoint for them. Any Python-based Portage
scripts they execute are now tried from the same directory first and
will therefore use the correct environment, as above.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
It was breaking the Portage sandbox.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
@thesamesam
Copy link
Member

I've merged this so we can get it some more testing - that doesn't mean more comments and such aren't welcome, obviously. Thank you!!

palao pushed a commit to palao/portage that referenced this pull request Oct 16, 2023
It was breaking the Portage sandbox.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
Closes: gentoo#1071
Signed-off-by: Sam James <sam@gentoo.org>
palao pushed a commit to palao/portage that referenced this pull request Oct 22, 2023
It was breaking the Portage sandbox.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
Closes: gentoo#1071
Signed-off-by: Sam James <sam@gentoo.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants