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

List of patches unclear, Xapian source code in the repository #2

Open
kelson42 opened this issue Aug 18, 2024 · 7 comments
Open

List of patches unclear, Xapian source code in the repository #2

kelson42 opened this issue Aug 18, 2024 · 7 comments
Assignees
Labels
question Further information is requested

Comments

@kelson42
Copy link

kelson42 commented Aug 18, 2024

How do we get the list of patches appied to Xapian source code?

Considering we should not have to patch the source code to compile it, each patche should be clearly identified and a strategy to bring it upstream shoukd be available.

@kelson42 kelson42 added the question Further information is requested label Aug 18, 2024
@kelson42 kelson42 pinned this issue Aug 18, 2024
@mgautierfr
Copy link
Contributor

This is a git repository. All patches are in form of commits in git history.
But anyway, I will soon write a resume of the status about compilation on Windows, probably on kiwix-build side.

@kelson42
Copy link
Author

@mgautierfr We should not have the Xapian source code here. I'm very concerned as this breaks our internal rules and best practices, which leads to a serious maintainability challenge. Any reason why these patches are not applied as patch on the original source code (to be downloaded)?

Current situation should be understood as temporary and goal is to remove this source code. As such, the required issues should be open here so we can track each of this modification/patches, or modification groups. One per issue. For each issue, a PR should be made upstream to request the change or a justification should be given to keep it.

@kelson42 kelson42 changed the title List of patches List of patches unclear, Xapian source code in the repository Aug 19, 2024
@mgautierfr
Copy link
Contributor

Current situation should be understood as temporary and goal is to remove this source code.

We agree on that

Any reason why these patches are not applied as patch on the original source code (to be downloaded)?

Because we are in a temporary (Work in Progress) situation and we want to track the evolution of the patches applied. Having a temporary repository with all added file versionned on top of xapian source code is really simpler

For each issue, a PR should be made upstream to request the change or a justification should be given to keep it.

90% of the current commit are about meson compilation only.
Some patched has been already applied upstream (and this also why we are based on git upstream and not on released version which doesn't contain such patches)
If not send upstream, it is mainly because of priority issue.

@kelson42
Copy link
Author

@mgautierfr It's important that - before your absence - that:

  • Situation is more or less clean, ie. no Xapian source codes and proper unit patches applied. It should have been like this for the very beginning.
  • Work left to do, here moving patches upstream, is clear and identified in atomic tickets both here and upstream.

@mgautierfr
Copy link
Contributor

Situation is more or less clean, ie. no Xapian source codes and proper unit patches applied. It should have been like this for the very beginning.

I disagree with you.

  • No xapian source codes: This is the best way to share patches with upstream and this is what we ultimately want. We have already shared patches this way and we will probably continue to do so.
  • Proper unit patches applied: We are still in work in progress. Until we have fixed issue and ready to do a PR "upstream" (either on xapian or meson wrapbd), I prefer to keep all intermediate commit which may contains useful information instead of squash them (especially for my future me finish this)

@kelson42
Copy link
Author

kelson42 commented Aug 30, 2024

Here are the things I want to see followed:

  • No source code of third party software in our repos without my explicit approval. This has always been like that.
  • Here there is no good reason to have it except been "lazy" at the cost of the others (for example clarity about what/why/how things are done) and reproducibility (needed for Needs README and LICENSE files #3)
  • A commit is not a patch to me, I want list of patch properly organised in dedicated files and applied in a similar manner like at kiwix-build
  • Any of this patch - which should be ultimately put upstream - should have it's own issue to track the effort of upstreaming with a directly link the upstream issue.
  • Commits are mainly for developers. I rarely look to the commits themselves. I move at the issue/PR/milestone level and documentation has to be made there as well with the correct level of granularity and adaptation to the audience.

I understand that this is a work in progress but:

  • The approach (modifying directly the source code) like you made, can perfectly be needed at early stage, but keep in mind that this can be on your local system or personal repository, but this is not the level of quality expected in official Kiwix repositories.
  • Issues are here to track the work left to do and this has to be done properly. I you know you have work left to do, you have to create an issue.
  • After months and months of work we are now hitting the deadline. Therefore the MS Windows CI/CD instrumentation has to be clean and what should be left to do are things which depend on third parties AND refinements which were not foreseen OR mandatory.

@mgautierfr
Copy link
Contributor

The approach (modifying directly the source code) like you made, can perfectly be needed at early stage, but keep in mind that this can be on your local system or personal repository, but this is not the level of quality expected in official Kiwix repositories.

I wanted to create a repository on my personal account but you ask to have it in openzim organization...


I propose the following:

  • Move this repository in my account
  • make kiwix-build using xapian official release + set of "classic" patches
  • Thoses patches would be store (and versioned) in kiwix-build (as any other patches we use for other project)
  • Move xapian-meson project issues in kiwix-build as this repository would be useless.
  • Delete/Archive this repository

Is it ok for you ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants