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

version based on git #229

Merged
merged 4 commits into from Jul 30, 2021
Merged

version based on git #229

merged 4 commits into from Jul 30, 2021

Conversation

dalf
Copy link
Member

@dalf dalf commented Jul 27, 2021

What does this PR do?

This commit removes:

  • the need to update the brand for GIT_URL and GIT_BRANCH.
  • these settings:
    # If you change a value below don't forget to rebuild instance's enviroment
    # (make buildenv)
    git_url: https://github.com/searxng/searxng
    git_branch: master

these values are read from the git repository: it requires the directory to be git repository.

It is possible to call python -m searx.version freeze to freeze the current version (it creates the searx/version_frozen.py file)
It is useful when the code is install outside git (distro package, docker, etc...)

The code is based on setuptool_scm: it calls the git command line to retrieve the values. See searx/version.py.

Why is this change important?

With the change, the footer is always up to date without additional configuration:
image

  • 1.0.0-590-f2979c72 comes from git describe
  • -dirty shows that there are some uncommited changes.

How to test this PR locally?

  • make run: check the version at the bottom of the page is the right one.
  • create a tag locally, check the version is still right.
  • call python -m searx.version freeze
    • add some change
    • check that version is same as before the changes
  • make docker:
    • check the version at the bottom of the page is the right one.
  • make docs
  • use utils/searx.sh to install the app
    • check the version
    • check the utils/searx.sh update works

Author's checklist

Related issues

Closes: #36

searx/version.py Outdated Show resolved Hide resolved
@return42
Copy link
Member

Just for my understanding, where do you see the cutting line of installation procedure and the repository which will be installed?

On the long run, we want to isolate all the installation scripts (e.g. ./utils/searx.sh) from the SearXNG repository.

Lets imagine we have a command line named install_searxng and it's usage is like:

install_searxng <git-url> <git-branch-name>

With I can suppose two installation types:

  1. Those directly install the master branch from https://github.com/searxng
  2. Those installing a branch from another repository / e.g. like I do, I install branch darmarit.org from my fork https://github.com/return42/searxng

Now my question is: where does GIT_URL and GIT_BRANCH belongs to? Should these values in the repository or should this values stored somewhere in the installation?

If I understand this Draft correct, these values are stored in a new file ./searx/version_frozen.py. Is this file a part of the repository (will it be committed to the fork) or not .. and what does it mean .. what about this file when updating an existing SearxNG installation?

Having both installation types (from above) and an update process in mind, I always revolve around this point and can't get any further.

Can you describe what scenarios you have in mind and what solution you suppose.

@dalf
Copy link
Member Author

dalf commented Jul 28, 2021

Should these values in the repository or should this values stored somewhere in the installation?

That is the point of this PR : searx/version_frozen.py is not commited (neither in this git repository nor the forks).
The git repository already has this information and when searxng starts , it calls the git command to get the GIT_URL, GIT_BRANCH, VERSION_STRING.

Actually, there are two modes:

  • running searxng from a git repository (this repository or another one): version_frozen.py is not required. version.py will get the GIT_URL, GIT_BRANCH, VERSION_STRING from the git repository (using the git command). In this mode, the searxng installation is updated using git pull : searxng will read the VERSION_STRING value when it starts.
  • running searxng outside a git repository ( debian, arch, yunohost packages ):
    • the files have to come from somewhere and they comes from a git repository at one point or another. So it is possible to call python -m searx.version freeze to create searx/version_frozen.py. After that the version is hard coded, the git command line is not called anymore.
    • then it is possible to copy / archive the files .

If you run searxng from your fork, GIT_URL will be your fork URL.
Technically, version.py will:

  • get the upstream reference of the current branch git rev-parse --abbrev-ref @{upstream} (if the current branch doesn't an upstream, it will check the master branch).
  • get the remote URL: git remote get-url {origin}

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.

@dalf thanks for your explanation / sounds good to me .. but to be true I need some practice to see if there is a trap I don't see right now. Anyway, lets go into this direction ... I made some remarks in my review .. I think the version.py can be improved.

If you want, I can try to improve the implementation .. but I cant promise you when.

utils/searx.sh Outdated Show resolved Hide resolved
searx/version.py Outdated Show resolved Hide resolved
searx/version.py Outdated Show resolved Hide resolved
searx/version.py Outdated Show resolved Hide resolved
searx/version.py Outdated Show resolved Hide resolved
searx/version.py Outdated Show resolved Hide resolved
searx/version.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
searx/version.py Outdated Show resolved Hide resolved
searx/version.py Outdated
Comment on lines 33 to 35
def call_subprocess(cmd, cwd="."):
if os.name == "posix" and not isinstance(cmd, (list, tuple)):
cmd = shlex.split(cmd)
Copy link
Member

Choose a reason for hiding this comment

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

sorry if I'm being annoying :-) .. What use case (OS) do you have in mind for non posix .. and what happens with the sub process call when os.name != "posix"?

Copy link
Member Author

@dalf dalf Jul 29, 2021

Choose a reason for hiding this comment

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

My take was setup_scm has most probably dealt with various platforms.

But, more in detail, from https://docs.python.org/3/library/subprocess.html#subprocess.Popen :

On Windows, if args is a sequence, it will be converted to a string in a manner described in Converting an argument sequence to a string on Windows. This is because the underlying CreateProcess() operates on strings.

And according to https://stackoverflow.com/questions/1854/python-what-os-am-i-running-on os.name != 'posix' only on Windows (and Jython which is out of scope and perhaps some other implementation I'm not aware of, yes it should be os.name != 'nt' according to what I understand even in setup_scm).

But anyway searx/version.py works on Windows even with the commit 7710c94

@return42 return42 self-requested a review July 29, 2021 18:52
@dalf dalf changed the title [WIP / RFC] version based on git version based on git Jul 29, 2021
searx/version.py Outdated Show resolved Hide resolved
@return42
Copy link
Member

FYI: alternative version.py implementation (including last patches from here) https://github.com/return42/searxng/commits/version-from-git

@return42
Copy link
Member

return42 commented Jul 30, 2021

Actually, there are two modes:

  • running searxng from a git repository (this repository or another one): version_frozen.py is not required. version.py will get the GIT_URL, GIT_BRANCH, VERSION_STRING from the git repository (using the git command). In this mode, the searxng installation is updated using git pull : searxng will read the VERSION_STRING value when it starts.
  • running searxng outside a git repository ( debian, arch, yunohost packages ):
    • the files have to come from somewhere and they comes from a git repository at one point or another. So it is possible to call python -m searx.version freeze to create searx/version_frozen.py. After that the version is hard coded, the git command line is not called anymore.
    • then it is possible to copy / archive the files .

Hm, now I see what I missed .. this is run-time, but I asked for the installation-time:

With I can suppose two installation types:

  1. Those directly install the master branch from https://github.com/searxng

What I have tried / with the fixes in my branch

1.1. clone https://github.com/searxng/searxng
1.2. edit etc/settings.yml (e.g. edit server.port from 8888 to 7777)
1.3. make buildenv
1.4. install complete suite::

 ./utils/searx.sh install all
 ./utils/filtron.sh install all
 ./utils/filtron.sh nginx install
 ./utils/morty.sh install all
  1. Those installing a branch from another repository / e.g. like I do, I install branch darmarit.org from my fork https://github.com/return42/searxng

2.1. clone (e.g.) https://github.com/return42/searxng
2.2. checkout branch (e.g.) version-from-git
2.3. install complete suite::

 ./utils/searx.sh install all
 ./utils/filtron.sh install all
 ./utils/filtron.sh nginx install
 ./utils/morty.sh install all

How to proceed? I would suggest to reset your branch to the state of mine. Otherwise I have to repeat all tests in your branch (I do not have more time today).

My branch is based on your e5d1cd3 and includes your patch 2d60d80 (and 7710c94 is not needed in my version.py).

What I mean, everything from your branch (except 93e3e3f) is in my fully tested branch. I prefer my branch since 93e3e3f in your branch means that I have to repeat all my tests.

Is it OK for you to reset your branch with mine?

Afterwards we can squash some commits / I think.

@dalf
Copy link
Member Author

dalf commented Jul 30, 2021

I would like to understand what are the fixes that you have implemented: I don't understand what is not working?

I would like to know how to test? Just the command you have provided?

(I'm installing a VM to run the scripts)

@return42
Copy link
Member

return42 commented Jul 30, 2021

would like to understand what are the fixes that you have implemented: I don't understand what is not working?

my branch starts at your commit e5d1cd3 and places some patches on top. In detail:

Until here both branches are more or less identical, while 93e3e3f is untested return42@5d7757d is tested.

The next commits are not in your branch, they are only in my branch, fixing some issues I had at installation time ...

Sorry if this is a mess of two branches but we developed parallel in two branches. You merged parts from mine and I merged parts from yours :-)

dalf and others added 4 commits July 30, 2021 14:40
This commit remove the need to update the brand for GIT_URL and GIT_BRANCH:
there are read from the git repository.

It is possible to call python -m searx.version freeze to freeze the current version.
Useful when the code is installed outside git (distro package, docker, etc...)
The first import of names from the searx package implies loading the
settings.yml.  Before this is done, the enviroment variables must be unset to
not overwrite the values from the settings.yml

Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
Fix the quotation marks in the command line to run

    python -m searx.version

Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
@return42
Copy link
Member

If you think PR is ready to review, remove the "Draft" tag .. then I will run my tests this evening.

@return42
Copy link
Member

return42 commented Jul 30, 2021

I would like to know how to test? Just the command you have provided?

Yes. Frst I create a LXC:

sudo -H ./utils/lxc.sh remove searx-archlinux
sudo -H FORCE_TIMEOUT=5 ./utils/lxc.sh build searx-archlinux

Then I jump in the container ..

sudo -H ./utils/lxc.sh cmd searx-archlinux bash

in this bash in the container I run the installation procedures shown in #229 (comment)

I have tested only in archlinux but others should be the same.

@return42 return42 self-requested a review July 30, 2021 15:56
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.

Works flawless / lets merge.

BTW: Thank you for your patience with me :-)

@dalf dalf marked this pull request as ready for review July 30, 2021 16:56
@dalf dalf merged commit 38ee88b into searxng:master Jul 30, 2021
@dalf dalf deleted the version-from-git branch July 31, 2021 08:03
@dalf
Copy link
Member Author

dalf commented Jul 31, 2021

@paulgoio, since this PR has merged, it seems you have to copy all the files from this repository to update your fork?
If you have an issue feel free to create open one.

@HLFH
Copy link
Contributor

HLFH commented Sep 29, 2021

@dalf

You said:

It is possible to call python -m searxng.version freeze to freeze the current version (it creates the searxng/version_frozen.py file). It is useful when the code is installed outside git (distro package, Docker, AUR etc...).

Can the command python -m searxng.version freeze be in the documentation?
Otherwise, without this relevant info, people stay with an about page having broken links for SearXNG sources.

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.

None yet

3 participants