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

Only complain about older config version if there was a change #3074

Conversation

bazsi
Copy link
Collaborator

@bazsi bazsi commented Jan 13, 2020

Resolves #3068

On syslog-ng upgrade, if the @version: number in the config file left on the older version, a warning WARNING: Configuration file format is too old, syslog-ng is running in compatibility mode. Please update it to use the syslog-ng ... can be seen.

This PR makes it possible that a syslog-ng upgrade does not have to be followed by a version number upgrade in the config file, except there is a change in compatibility.
In a lot of cases there is no compatibility changes between 2 versions, so this saves some unnecessary config editing.

It's important, that users should still use the installed syslog-ng version number in their configuration,
even if the syslog-ng -V Config Version reports the version which is compatible with.

@kira-syslogng
Copy link
Contributor

Build FAILURE

lib/versioning.h Outdated Show resolved Hide resolved
Copy link
Collaborator

@gaborznagy gaborznagy left a comment

Choose a reason for hiding this comment

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

Approve besides the one change request about VERSION_CURRENT.

I favor if we don't make false warning messages, which is solved with this PR.
Prior this PR having the "compatibility mode" warning was just a last step reminder, we have warning messages everywhere where backward compatibility is broken.

In other words, the false warning message can be resolved also by removing the warning message about "compatibility mode". There is no action in cfg.c, every individual action to ensure compatibility is done at the given place, so this is just another reminder message to check the other warning messages.

@bazsi bazsi force-pushed the only-complain-about-older-config-version-if-there-was-a-change branch from 809df55 to 3e5193b Compare January 20, 2020 13:27
@bazsi
Copy link
Collaborator Author

bazsi commented Jan 20, 2020

removed a compile error (sorry for not noticing) and also added a G_STATIC_ASSERT() to check
if VERSION_LAST_SEMANTIC_CHANGE wasn't updated properly.

@kira-syslogng
Copy link
Contributor

Build SUCCESS

1 similar comment
@kira-syslogng
Copy link
Contributor

Build SUCCESS

gaborznagy
gaborznagy previously approved these changes Jan 20, 2020
Copy link
Collaborator

@gaborznagy gaborznagy left a comment

Choose a reason for hiding this comment

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

I like that we have a check now.

lib/cfg.h Outdated Show resolved Hide resolved
@gaborznagy
Copy link
Collaborator

@bazsi Yesterday we discussed this topic and stated that we should clearly separate config version from the actual version of syslog-ng.

This means that syslog-ng -V output with Config version: could show the last semantic version, i.e. the config version.

I've shared my personal opinion on this topic before, and I would like to have some feedback on it both from you and the team (reviewers).

... from config version point of view there is no difference between 3.22 and 3.25, but I think upgrading syslog-ng itself to 3.25 and then setting the config version to 3.22 gives a false sense of an LTS version.
See #3074 (comment)

@bazsi bazsi force-pushed the only-complain-about-older-config-version-if-there-was-a-change branch from a52275b to a70d649 Compare January 30, 2020 10:41
@bazsi
Copy link
Collaborator Author

bazsi commented Jan 30, 2020

I've just rebased/updated the branch to display the last semantic change in config-version. I did some cleanups in the naming of macros in versioning.h as I tended to forget which macro I needed and by the time I've checked it out and got it to the place I wanted to use it, I forgot it again.

The conventions I introduced are documented in the versioning.h file.

Also I changed the "Config-Version" in the --version output to use a dash in place of the space, to match conventions in that output. I don't think this has been parsed by tools until now.

Sidenote (but not included in this branch): I found that we are using SYSLOG_NG_SOURCE_REVISION at a number of places but it is not set in automake based builds, so we can quite possibly eliminate that now as the version number does contain the git commit id (or we should set it to the git commit id as it was originally intended). I

@kira-syslogng
Copy link
Contributor

Build SUCCESS

@gaborznagy gaborznagy self-requested a review January 30, 2020 12:06
syslog-ng/main.c Outdated Show resolved Hide resolved
lib/versioning.h Show resolved Hide resolved
gaborznagy
gaborznagy previously approved these changes Feb 11, 2020
Copy link
Collaborator

@gaborznagy gaborznagy left a comment

Choose a reason for hiding this comment

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

If the "Config Version" output change is reverted and the tests turn green, this can go in.

@bazsi bazsi force-pushed the only-complain-about-older-config-version-if-there-was-a-change branch from a70d649 to 17b77f7 Compare February 13, 2020 10:54
@bazsi
Copy link
Collaborator Author

bazsi commented Feb 13, 2020

rebased, and changed the "config version" output to use a space instead of a dash.

@kira-syslogng
Copy link
Contributor

Build SUCCESS

@bazsi bazsi force-pushed the only-complain-about-older-config-version-if-there-was-a-change branch from 17b77f7 to 51db0ab Compare February 17, 2020 07:17
@bazsi
Copy link
Collaborator Author

bazsi commented Feb 17, 2020

Rebased and added news entry.

@kira-syslogng
Copy link
Contributor

Build SUCCESS

gaborznagy
gaborznagy previously approved these changes Feb 17, 2020
@lbudai lbudai added this to the syslog-ng-3.26 milestone Feb 19, 2020
We already have defines for version numbers through versioning.h, use them.

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
This was only used for initializing ModuleInfo->core_revision, but for that
purpose we were using SYSLOG_NG_SOURCE_REVISION, which was supposed to
be a git commit ID, but today it is unset (in automake) or set to
$VERSION (in cmake). That field is completely unused now, so should
probably be removed completely.

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
The different macros became a mess (and I am about to introduce another
bunch), so do some cleanups.

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
…ange

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
@gaborznagy gaborznagy force-pushed the only-complain-about-older-config-version-if-there-was-a-change branch from 51db0ab to 6c41a97 Compare February 21, 2020 10:48
@kira-syslogng
Copy link
Contributor

Build FAILURE

@gaborznagy
Copy link
Collaborator

@kira-syslogng retest this please;

@gaborznagy
Copy link
Collaborator

@bazsi I've resolved the merge conflict and pushed it.

@kira-syslogng
Copy link
Contributor

Build SUCCESS

@MrAnno MrAnno merged commit a519190 into syslog-ng:master Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
user-visible-feature User visible feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default RPM update behavior forces compatibility mode
8 participants