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

Setup.py: fix infopage location #3045

Merged
merged 1 commit into from
Feb 24, 2024

Conversation

jwijenbergh
Copy link

What does this PR do?

Previously the infopage was under searx/info directory, it seems like this was never changed

Why is this change important?

About page gives a 404. This doesn't affect the docker image because that just copies over the whole searx directory.

How to test this PR locally?

Build a wheel and see that it doesn't contain the info page

Author's checklist

Related issues

Don't think this has been reported yet

Copy link
Member

@return42 return42 left a comment

Choose a reason for hiding this comment

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

Don't think this has been reported yet

Since we do not build the searx package and some of the items (e.g. test) do not exists, I assume we can drop package_data (its a ten year old leftover :) ..

searxng/setup.py

Lines 54 to 75 in 8a4104b

package_data={
'searx': [
'settings.yml',
'../README.rst',
'../requirements.txt',
'../requirements-dev.txt',
'data/*',
'info/*',
'info/*/*',
'plugins/*/*',
'static/*.*',
'static/*/*.*',
'static/*/*/*.*',
'static/*/*/*/*.*',
'static/*/*/*/*/*.*',
'templates/*/*.*',
'templates/*/*/*.*',
'tests/*',
'tests/*/*',
'tests/*/*/*',
'translations/*/*/*'
],

@jwijenbergh
Copy link
Author

Don't think this has been reported yet

Since we do not build the searx package and some of the items (e.g. test) do not exists, I assume we can drop package_data (its a ten year old leftover :) ..

searxng/setup.py

Lines 54 to 75 in 8a4104b

package_data={
'searx': [
'settings.yml',
'../README.rst',
'../requirements.txt',
'../requirements-dev.txt',
'data/*',
'info/*',
'info/*/*',
'plugins/*/*',
'static/*.*',
'static/*/*.*',
'static/*/*/*.*',
'static/*/*/*/*.*',
'static/*/*/*/*/*.*',
'templates/*/*.*',
'templates/*/*/*.*',
'tests/*',
'tests/*/*',
'tests/*/*/*',
'translations/*/*/*'
],

Hmmm the reason I found out is because I use the package on nixos. That would thus break packages like that as those files would then not be packaged at all. But I understand if no one has the time to maintain it

@return42
Copy link
Member

One big problem I see is the name collision with the existing package https://pypi.org/project/searx/ .. which, as far I can say has never worked.

@dalf
Copy link
Member

dalf commented Dec 12, 2023

One big problem I see is the name collision with the existing package https://pypi.org/project/searx/ .. which, as far I can say has never worked.

It has worked, a long time ago.

I don't think name collision is a problem: if we published a package on pypi it should be named searxng, but there is no problem to include a module named searx inside this searxng package. This happen for some packages with C codes but without wheels, someone else publish another package with a different name but same module.

Exemple:

All these packages create a module named fasttext.

@return42
Copy link
Member

Hmmm the reason I found out is because I use the package on nixos.

AFAIK searx package has been dropped on NixOS

That would thus break packages like that as those files would then not be packaged at all.

I'm not an expert, but I don't think NixOS installs the python "searx" package from this repo here into the normal python packages .. if you have issues with the origin NixOS package maintained by the NixOS team @SuperSandro2000 please open a issue in the NixOS repository.

if we published a package on pypi it should be named searxng,

@dalf OK thanks for clarify / I think "pypi searxng packaging" will be a new feature and is not in the scope of this PR or what @jwijenbergh wanted to address .. I'm going to close this PR since it does not really solve an issue we have / as long we do not have a searxng package.

@return42 return42 closed this Dec 23, 2023
@jwijenbergh
Copy link
Author

jwijenbergh commented Dec 23, 2023

Hmmm the reason I found out is because I use the package on nixos.

AFAIK searx package has been dropped on NixOS

Yes the searx package, not the searxng package :/ https://search.nixos.org/packages?channel=23.11&from=0&size=50&sort=relevance&type=packages&query=searx

* [ searx: drop  NixOS/nixpkgs#258813](https://github.com/NixOS/nixpkgs/pull/258813)

That would thus break packages like that as those files would then not be packaged at all.

I'm not an expert, but I don't think NixOS installs the python "searx" package from this repo here into the normal python packages .. if you have issues with the origin NixOS package maintained by the NixOS team @SuperSandro2000 please open a issue in the NixOS repository.

It uses the setup.py to determine the package contents... Now we have to patch the package. So I thought the better fix was trying here in upstream first.

FWIW this patch makes me have a fully functioning package. But I understand if you want to close it, if so I would think the best way forward would be to drop the setup.py altogether

@return42
Copy link
Member

FWIW this patch makes me have a fully functioning package. But I understand if you want to close it, if so I would think the best way forward would be to drop the setup.py altogether

Not sure what to do next / first of all, I am reopening this PR to keep the discussion alive ..

I have not yet dealt with "how and whether" we should build a Python package ... for the following reason:

SearXNG is a rolling release / see Migrate and stay tuned!.

Typically it is installed by setting up a pyenv ...

searxng/utils/searxng.sh

Lines 490 to 494 in 6129b16

tee_stderr 0.1 <<EOF | sudo -H -u "${SERVICE_USER}" -i 2>&1 | prefix_stdout "$_service_prefix"
rm -rf "${SEARXNG_PYENV}"
python3 -m venv "${SEARXNG_PYENV}"
grep -qFs -- 'source ${SEARXNG_PYENV}/bin/activate' ~/.profile \
|| echo 'source ${SEARXNG_PYENV}/bin/activate' >> ~/.profile

... and installing it (in the pyenv) directly from the git clone. The installation from the shell script is a little bit more complex since we create a dedicated user for the SearXNG process and more .. but from the python's point of view, the installation is just a simple "developer" installation (therefore we need a setup.py):

pip install -e .

Since the "developer" installation is nothing more than a link, this installation does not require any packaging instructions and we can deploy updates by a git workflow.

searxng/utils/searxng.sh

Lines 570 to 577 in 6129b16

cd ${SEARXNG_SRC}
git fetch origin "$GIT_BRANCH"
git reset --hard "origin/$GIT_BRANCH"
pip install -U pip
pip install -U setuptools
pip install -U wheel
pip install -U pyyaml
pip install -U -e .

Since we have not published any other installation methods so far, no one has ever taken care of the packaging ... Packaging always requires release management and we don't have that in SearXNG because, as mentioned above, it is a rolling release.

I don't want to stand in anyone's way, but the only thing I would like to avoid is that the "back door" of packaging gives rise to the claim that SearXNG has releases.

I don't know how best to marry the rolling releases with the release management of a packager-manager like Linux distributors use (apt, pacman, nix ..).

Suggestions?


But think what we can (or should) do in any case is to delete the package_data completely first, because in this form it is wrong in any case.

searxng/setup.py

Lines 54 to 76 in 6129b16

package_data={
'searx': [
'settings.yml',
'../README.rst',
'../requirements.txt',
'../requirements-dev.txt',
'data/*',
'info/*',
'info/*/*',
'plugins/*/*',
'static/*.*',
'static/*/*.*',
'static/*/*/*.*',
'static/*/*/*/*.*',
'static/*/*/*/*/*.*',
'templates/*/*.*',
'templates/*/*/*.*',
'tests/*',
'tests/*/*',
'tests/*/*/*',
'translations/*/*/*'
],
},

@return42 return42 reopened this Dec 23, 2023
@SuperSandro2000
Copy link
Contributor

I have not yet dealt with "how and whether" we should build a Python package ... for the following reason:

It is already a python package. You can do python setup.py install and it mostly works.

SearXNG is a rolling release / see Migrate and stay tuned!.

From a NixOS perspective we don't really care. We just update to latest master branch once in a while and pick up the changes.

Typically it is installed by setting up a pyenv ...

We are more or less doing the same thing but the python installation is managed by nix, so people don't have to worry about state and things are reproducible and not depended on what was previously installed.

The installation from the shell script is a little bit more complex since we create a dedicated user for the SearXNG process and more

For NixOS that is all done in the searx(ng) module. We don't want packages to change system wide related things, so that the OS knows what happens and not a giant mess is created.

but from the python's point of view, the installation is just a simple "developer" installation (therefore we need a setup.py):

Just take away the -e and the package is done. :)

I don't think symlinks wouldn't work for nixos out of the box. We would need to rewrite them and I would rather avoid that.

Since we have not published any other installation methods so far, no one has ever taken care of the packaging ... Packaging always requires release management and we don't have that in SearXNG because, as mentioned above, it is a rolling release.

Packaging does not require a release workflow. We just take from master without any tags or uploads to pypi. Actually we usually prefer git downloads because the tarballs on pypi usually miss tests, sometimes source files for generated files or are not exactly created from the tag or are else somehow modified.

I don't want to stand in anyone's way, but the only thing I would like to avoid is that the "back door" of packaging gives rise to the claim that SearXNG has releases.

The current version in nixpkgs is unstable-2023-10-31, so taken from the date of the commit.

But think what we can (or should) do in any case is to delete the package_data completely first, because in this form it is wrong in any case.

Please don't. That's is truly all we need and We would revert it immediately anyway.

@return42
Copy link
Member

@SuperSandro2000 thanks a lot for your detailed feedback, given me some insights of NixOS packager 👍

Just take away the -e and the package is done. :)
I don't think symlinks wouldn't work for nixos out of the box. We would need to rewrite them and I would rather avoid that.

Its not a symbolic link what -e crates .. it creates an entry in a .pth file in the site-package of the pyenv.

Please don't. That's is truly all we need and We would revert it immediately anyway.

Do you have a chance to install it with -e? If not, we have to maintain package_data / I don't see a other way ..or?

@SuperSandro2000
Copy link
Contributor

Do you have a chance to install it with -e?

It is only supported in temporary nix-shells and we would probably need to expand upon that and add some extra logic to rewrite the pth file. I am not really wanting to plan to work on that.

https://github.com/NixOS/nixpkgs/blob/695f62353a45b8dd9a81b39f045a2a32bf38a8ab/doc/languages-frameworks/python.section.md#development-mode-development-mode

If not, we have to maintain package_data / I don't see a other way ..or?

Yeah, but that shouldn't be to hard. Not like There are new directories that often. I also think the current code could be replaced to just tell setuptools to just install all files in a directory, reducing the amount of wildcards it has.

@return42 return42 added the NixOS label Feb 23, 2024
@return42 return42 force-pushed the setup-fix-infopage branch 2 times, most recently from ff1c067 to 1fc1d5f Compare February 23, 2024 17:51
@return42
Copy link
Member

@jwijenbergh @SuperSandro2000 sorry for the very long delay .. I have lost sight of the PR / sorry.

I force pushed 1fc1d5f .. can you please test if it works for you / thanks 👍

Here is how/what I tested:

$ make clean py.build
...
$ python -m venv test123
$ . ./test123/bin/activate
(test123) $ pip install dist/searxng-2023.7.19*-py3-none-any.whl
(test123) $ SEARXNG_DEBUG=1 searxng-run

Open http://127.0.0.1:8888/info/en/about in your browser / do some tests ..

Copy link
Contributor

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

I didn't test much but the /info/en/about endpoint is now working on nixos

setup.py Outdated Show resolved Hide resolved
@SuperSandro2000
Copy link
Contributor

Here is how/what I tested:

Did this on NixOS:

diff --git a/pkgs/by-name/se/searxng/package.nix b/pkgs/by-name/se/searxng/package.nix
index a6a07a40adfd..7273552ca42f 100644
--- a/pkgs/by-name/se/searxng/package.nix
+++ b/pkgs/by-name/se/searxng/package.nix
@@ -1,6 +1,7 @@
 { lib
-, python3
 , fetchFromGitHub
+, fetchpatch
+, python3
 }:

 python3.pkgs.toPythonModule (python3.pkgs.buildPythonApplication rec {
@@ -14,6 +15,13 @@
     hash = "sha256-bThQgRcENoTIl4EwsjZiLT+f0X7pSV2Ly7X/HX1ECw0=";
   };

+  patches = [
+    (fetchpatch {
+      url = "https://github.com/searxng/searxng/pull/3045.patch";
+      hash = "sha256-sfzTtkCda6VdVPBtKXN8Ak/aeLuo2dI26S+v0vDu/HA=";
+    })
+  ];
+
   postPatch = ''
     sed -i 's/==.*$//' requirements.txt
   '';

Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
@return42 return42 merged commit d72fa99 into searxng:master Feb 24, 2024
9 checks passed
@jwijenbergh
Copy link
Author

jwijenbergh commented Feb 24, 2024

sorry for my late reply. It works perfectly, thank you!

return42 pushed a commit to return42/searxng that referenced this pull request Jun 17, 2024
Fix installing answerers when installing SearXNG through a wheel [1].  These
files have been missed in commit d72fa99.

Here is what have been tested:

    $ make clean py.build
    ...
    $ python -m venv test123
    $ . ./test123/bin/activate
    (test123) $ pip install dist/searxng-2024*-py3-none-any.whl
    (test123) $ SEARXNG_DEBUG=1 searxng-run

[1] searxng#3045 (comment)
return42 pushed a commit that referenced this pull request Jun 17, 2024
Fix installing answerers when installing SearXNG through a wheel [1].  These
files have been missed in commit d72fa99.

Here is what have been tested:

    $ make clean py.build
    ...
    $ python -m venv test123
    $ . ./test123/bin/activate
    (test123) $ pip install dist/searxng-2024*-py3-none-any.whl
    (test123) $ SEARXNG_DEBUG=1 searxng-run

[1] #3045 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants