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

nala: update to 0.11.1 #11843

Merged
merged 2 commits into from
Oct 31, 2022
Merged

nala: update to 0.11.1 #11843

merged 2 commits into from
Oct 31, 2022

Conversation

D-Brox
Copy link
Contributor

@D-Brox D-Brox commented Sep 7, 2022

I know this is a duplicate of 11337. I've already tried to make a PR with these updates on their fork, but it's been more than a month without new updates/commits. I've already discussed about updating the version with @volitank, the nala dev.

Please refer to #11337 for the original discussion around the patch improvements introduced in 0.11.0, aimed at termux.

@TomJo2000
Copy link
Member

Feel free to take this, I have not had any time to work on this.
I'm sorry, I shouldn't have committed to taking this on since I knew I'd have limited time with my new internship but I thought I'd be able to work on it.

@D-Brox
Copy link
Contributor Author

D-Brox commented Sep 7, 2022

Nala is building correctly, but I overlooked that installing from pyproject does not generate a .egg file. I'll update the script and squash the commits.

@D-Brox
Copy link
Contributor Author

D-Brox commented Sep 7, 2022

I also noticed the old build script uses termux_setup_python_crossenv. I'd like to ask if this step is really needed, considering that nala doesn't compile anything arch dependent.

Some simple python packages in the repo don't use this, while more complex ones do.

@TomJo2000
Copy link
Member

Could you also try to run this against nala 0.11.1?
It'd be helpful to see if this script would continue to work in the future, and potentially be able to do auto-updates.

@D-Brox
Copy link
Contributor Author

D-Brox commented Sep 7, 2022

Nala 0.11.1 was a hot-fix for a debian-only issue, and has not been added to the volian apt repo, so I can't really test it.

But now the package should be updatable by changing only the version and hash, in theory.

@TomJo2000
Copy link
Member

Ah, I hadn't looked into it that much.

LGTM. Thanks for your time.

@volitank
Copy link

volitank commented Sep 7, 2022

@TomJo2000

0.11.1 was only a small change to how we generate the date for the manpages during build time so that it is reproducible in Debian. This hot-fix changes nothing else for 0.11.0, the only thing is that the manpages will have the date from when it is built.

Considering that the termux build doesn't use the Debian packaging and instead uses pip there will be no differences at all.

Come to think of it though, it may be worth considering adding in translations, manpages and shell-completions to the install script.

for man pages these can be generated with ./nala_build.py man --install and the translations with ./nala_build.py babel --compile --install.

I don't know if these will put them in the right place, but it does use the termux path variable I added to make things easier. If not you can build them only by ommiting the --install switch, and then place them in the filesystem accordingly. If y'all end up trying this and install doesn't work, let me know and I'll update the build script for the next release.

For shell completions the files are Zsh: debian/_nala, Bash: debian/bash-completion, Fish: debian/nala.fish

Goal is to make Termux updates as painless as possible for everyone involved.

Also @D-Brox since they are using the GitLab Release page instead of my repo, all you have to do is change the version to 0.11.1 and it'll grab the source for that release.

@2096779623
Copy link
Member

Version 0.11.1 of nala has been released!

@TomJo2000
Copy link
Member

Version 0.11.1 of nala has been released!

As previously mentioned 0.11.1 is a hotfix release only concerning the man pages for Debian.
Although if it builds fine I don't see why we wouldn't use it, as it is the current version.

@D-Brox D-Brox changed the title nala: update to 0.11.0 nala: update to 0.11.1 Sep 12, 2022
@D-Brox D-Brox closed this Sep 12, 2022
@D-Brox D-Brox deleted the nala-0.11.0 branch September 12, 2022 16:26
@D-Brox D-Brox restored the nala-0.11.0 branch September 12, 2022 16:26
@D-Brox
Copy link
Contributor Author

D-Brox commented Sep 12, 2022

Sorry for the git log mess, with the closing and reopening, it was a missclick.
Anyway, I updated it to 0.11.1

@D-Brox D-Brox reopened this Sep 12, 2022
@D-Brox
Copy link
Contributor Author

D-Brox commented Sep 12, 2022

Also, could you answer #11843 (comment)? Since it could simplify the build script even further if the function is not needed

@TomJo2000
Copy link
Member

I just tested out the result from the latest CI run, it seems to work fine, but I had to manually create the $PREFIX/var/lock/ directory, otherwise it would crash with this message about being unable to create a lock file.

Traceback (most recent call last):
  File "/data/data/com.termux/files/usr/bin/nala", line 8, in <module>
    sys.exit(main())
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/nala/__main__.py", line 50, in main
    raise error from error
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/nala/__main__.py", line 41, in main
    nala()
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/typer/main.py", line 214, in __call__
    return get_command(self)(*args, **kwargs)
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/typer/main.py", line 532, in wrapper
    return callback(**use_params)  # type: ignore
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/nala/nala.py", line 236, in _update
    sudo_check()
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/nala/utils.py", line 368, in sudo_check
    NALA_LOCK_FILE.touch(exist_ok=True)
  File "/data/data/com.termux/files/usr/lib/python3.10/pathlib.py", line 1168, in touch
    self._accessor.touch(self, mode, exist_ok)
  File "/data/data/com.termux/files/usr/lib/python3.10/pathlib.py", line 331, in touch
    fd = os.open(path, flags, mode)
FileNotFoundError: [Errno 2] No such file or directory: '/data/data/com.termux/files/usr/var/lock/nala.lock'

This might be a local issue on my end, but will need to be investigated before this can be merged

@D-Brox
Copy link
Contributor Author

D-Brox commented Sep 13, 2022

This might be a local issue on my end, but will need to be investigated before this can be merged

I guess the dir can be created in the postinst debscript, along with the other ones, of it doesn't exist. Tho that dir probably exists in most cases, since it's commonly used for lock files.

@TomJo2000
Copy link
Member

I guess the dir can be created in the postinst debscript, along with the other ones, of it doesn't exist. Tho that dir probably exists in most cases, since it's commonly used for lock files.

Yeah, as I said, probably a local issue, it would still be best to create the directory if it is missing to prevent such an issue.

@D-Brox
Copy link
Contributor Author

D-Brox commented Sep 20, 2022

I was testing my patches directly on termux, seems like I need to add a pip dependency (python-debian) topyproject.toml, as it's not being installed by default.

@D-Brox D-Brox force-pushed the nala-0.11.0 branch 2 times, most recently from 10c4a94 to 4456625 Compare October 8, 2022 22:24
@D-Brox D-Brox requested a review from Grimler91 October 8, 2022 22:25
@Grimler91
Copy link
Member

Using termux_setup_python_crossenv sounds like a better idea to me, a package shouldn't really modify the host files. If a user would abort the build then they would have a broken sysconfig.py file on their computer

@D-Brox
Copy link
Contributor Author

D-Brox commented Oct 9, 2022

Using termux_setup_python_crossenv sounds like a better idea to me, a package shouldn't really modify the host files. If a user would abort the build then they would have a broken sysconfig.py file on their computer

Yes, I noticed that. I ended up using a normal virtualenv instead of the crossenv, since it proved to be way faster while achieving that same desired result.

@D-Brox
Copy link
Contributor Author

D-Brox commented Oct 11, 2022

@Grimler91 I think the update is now finally ready.

What's been done:

  • The patches were simplified following the modifications made by @volitank to increase termux compatibility.
  • Since nala now uses a pyproject.toml instead of a setup.py file, the installation method was simplified and doesn't use the deprecated python egg as before.
  • pip installs the deps from pyproject.toml packages by default, but those shouldn't be packaged inside the deb. The build script installs python deps with pip on the postinst script, as it was doing before.
  • To circunvent the distro dependent sysconfig.py issue, which makes pip install in /local/lib/python3.X/dist-packages, I used a dummy virtualenv to "trick" python into always installing in /lib/python3.X/site-packages.
    The venv from termux_setup_python_crossenv could also have been used, but it proved to be slower.
  • I also removed all unused variables, functions and patch files.

The next updates will probably not take this long, since they'll hopefully only change the version and hash (at least until the rust rewrite is released).

Copy link
Member

@Grimler91 Grimler91 left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, please update the commit message so that it is prefixed with nala:, so that it is easy to see which packages it is that is changed

scripts/properties.sh Outdated Show resolved Hide resolved
packages/nala/build.sh Outdated Show resolved Hide resolved
packages/nala/build.sh Outdated Show resolved Hide resolved
@D-Brox D-Brox requested a review from Grimler91 October 23, 2022 03:31
@D-Brox D-Brox force-pushed the nala-0.11.0 branch 7 times, most recently from ea76f29 to 0d112b1 Compare October 30, 2022 15:32
@D-Brox
Copy link
Contributor Author

D-Brox commented Oct 30, 2022

@buttaface @Grimler91 @landfillbaby @2096779623
I'm taking the liberty to ping all termux reviewers as, for some reason, only Grimler91 is listed as a reviewer on this PR:
image

@Grimler91
Copy link
Member

@xtkoba argued in #12576 (comment) that we should use termux_setup_python_crossenv for all python packages, so could you please add usage of that function again

@D-Brox
Copy link
Contributor Author

D-Brox commented Oct 30, 2022

@Grimler91 Anything else?

@Grimler91
Copy link
Member

@Grimler91 Anything else?

Otherwise it looks good to me, if CI build succeeds and files are installed to correct place. Will merge in ~12 h unless someone else has comments, thanks

D-Brox and others added 2 commits October 31, 2022 08:49
@Grimler91 Grimler91 merged commit 4a0717d into termux:master Oct 31, 2022
@Grimler91
Copy link
Member

Thanks!

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.

5 participants