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

fix build and NPE on master #5294

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

line-o
Copy link
Member

@line-o line-o commented Apr 30, 2024

Description:

This PR is deliberately targeting the master branch and does two things:

  1. it fixes the build of exist-db [BUG] Building on master is broken #4911
  2. it fixes the NPE when an XML-resource is defragmented [BUG] NPE thrown on frequent updates of an XML resource #5273

Reference:

Backport of #5276
fixes #4911
fixes #5273

Type of tests:

One jUnit test was added that forces defragmentation of a resource on first update to check if the process finishes and the update went through.

- add constructor to ExistXmldbEmbeddedServer that allows passing in configuration properties
- create new test that updates a document with fragmentation limit set to -1
A regression introduced in c553667 causes a NullPointerException to be thrown whenever an XML-resource
needs to be defragmented.
This will be triggered on frequent writes to any XML-resource and will effectively remove the file
from the database and from any indexes.
@line-o line-o requested review from a team and dizzzz April 30, 2024 16:33
Copy link
Contributor

@adamretter adamretter left a comment

Choose a reason for hiding this comment

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

We never commit to master directly. As per the documentation, master just points to the latest release commit. Please close this.

@line-o line-o requested review from reinhapa and a team April 30, 2024 16:37
@dizzzz
Copy link
Member

dizzzz commented Apr 30, 2024

The NPE I agree on.... that is a bug-fix for the app;
The other is a build fix?

@line-o
Copy link
Member Author

line-o commented Apr 30, 2024

The only alternative that I see is that we create a release branch: release-6.2.1

@adamretter
Copy link
Contributor

adamretter commented Apr 30, 2024

please could you then indicate how this should be done?

@dizzzz Sure, it's in the documentation - https://github.com/eXist-db/exist/blob/develop/exist-versioning-release.md#release-process

We have no idea what is acceptable for you...

I find that a bit of a strange and jarring comment... This is not at all about what is acceptable to me, it is about following the rules that we all reviewed, agreed to, and documented during an open community process. Those rules were created to make sure that we have a consistent, working, and repeatable process for doing the releases.

I recall that before we documented the process very few people knew how to do a release, doing so was very time consuming and difficult, and each one looked a bit different to the previous one.

This release process document has been in place since 2017 (or earlier?) and every release has followed those instructions since (https://github.com/eXist-db/exist/commits/develop/exist-versioning-release.md).

@adamretter
Copy link
Contributor

adamretter commented Apr 30, 2024

The only alternative that I see is that we create a release branch: release-6.2.1

If you follow the documented release process you should not need to create a release branch, you can just tag the appropriate commit.

@line-o
Copy link
Member Author

line-o commented Apr 30, 2024

We need to get a patch release out to fix a fatal regression resulting in data loss.
We cannot release from develop-6.x.x (6.3.0-SNAPSHOT) nor develop (7.0.0-SNAPSHOT) because both have too many changes already merged into them.
I need to know what the accepted way forward is to be able to release 6.2.1 as the next stable release.

@dizzzz
Copy link
Member

dizzzz commented May 1, 2024

@adamretter

I find that a bit of a strange and jarring comment...

That was not my intention, apologies, but clearly we (me) need some (more) guidance and would like to fix some long-time issues.

Copy link
Member

@dizzzz dizzzz left a comment

Choose a reason for hiding this comment

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

based on feedback... we need to have an other route

@line-o
Copy link
Member Author

line-o commented May 2, 2024

@adamretter the document you refer to only mentions the develop branch which is not suitable to create a 6.2.1 as it contains a snapshot of version 7 and contains breaking changes.
There is no mention of the development branches for earlier versions. But as I already wrote develop-6.x.x does already contain quite a few changes and might therefore not be suitable either.
@dizzzz I will keep this open until we decided how to create a patch release

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.

3 participants