pre and post 1.7.0 log support

[donkers]

Based on the info I found at https://www.hosthorde.com/forums/resources/understanding-minecraft-server-log-files.75/ I believe this should support the log format of at least all release Minecraft versions.

Tested with Minecraft versions 1.2.5, 1.3.2, 1.6.4, 1.7.10, 1.8.8

[renderorange]

Hello, thanks for the pull request. I'm going to be doing some testing on this tonight.

[renderorange]

I went ahead and did some testing on your pull request and there is one thing I initially see.

The new versioning file for minecraft needs to be added to the versioning manifest file.

versioning/versions.txt

then, the following line

minecraft/1.7.0

this change will allow for the msm update command to download the 1.7.0.sh file to load the new regex. Without that line, on installation or update, msm won't download the new file to support the changes.

I'm still in the process of testing, but haven't been able to get through all of the things I'd like to test tonight.

If you wouldn't mind updating the above for your pull request, thanks :)

[donkers]

Hey, Sorry, didn't think to check the updating mechanism. I will push another commit with the requested changes when I get home from work, if not before.

[renderorange]

Thanks a lot.

In testing, another issue appears to be the log file location. Even though the regex is correct, it's not reading the log file.

I added a testing block into the server_log_dots_for_lines function.

Starting server...
# regex: ^\[[0-9]{2}:[0-9]{2}:[0-9]{2}\] \[.*\]: (Done) #
# log path: /opt/msm/servers/stable/server.log #
 Done.

This server is running the latest stable for minecraft server, 1.7.0 set in server.properties.

Changing the log file name in /etc/msm.conf appears to fix this and allow for the proper reading of the file

#DEFAULT_LOG_PATH="server.log"
DEFAULT_LOG_PATH="logs/latest.log"

after that, when starting msm, the following is output

Starting server...
# regex: ^\[[0-9]{2}:[0-9]{2}:[0-9]{2}\] \[.*\]: (Done) #
# log path: /opt/msm/servers/stable/logs/latest.log #
.......... Done.

The dots are then properly output, Done displayed, and quickly exited.

The quick fix would be to change the DEFAULT_LOG_PATH variable within the master branch, which would allow for new installs to utilize the new code. More than likely, this is what most admins are doing already, adjusting for the new log path.

Without the LOG_PATH change, and with the msm-version set for 1.3.0.sh, the log path matches for minecraft server 1.5.3. The issue would appear when admins are running multiple server versions. For both 1.5.3 and 1.8.8, one will work and the other won't.

The question is, would it be worth additional work to move this variable out into the version file as well? What I gather as your main purpose is to allow for both legacy and new log file formats together, so having to hard code the log file location for each one seems to be counter to this idea.

Thoughts on this?

[donkers]

Sounds like we are on the same page about that. I was planning on moving the LOG_PATH as well, for all the reasons you have covered, I just haven't had the time as of yet. ...maybe next weekend.

[marcuswhybrow]

I'd vote for adhering to the expectation. If the log location changes with version, I'd expect it to be a version-able property.

[marcuswhybrow]

Versioning has only two declarative options: console_command and console_event. We'd need a to do two things to allow server properties to be set by versioning files:

1. Add a hook for setting properties in versioning files:

   # $1: The name of the property
   # $2: The value of the property
   set() {}
   

2. Rework server_property to also check versioning files declarations of properties if not found in server.properties.

---

The logic flow in server_property is a bit random:

• It's assumed version files can only declare a special group of property CONSOLE_* properties (#L1793).
• If the property is of that class it will check the version file and not check server.properties (#L1803).
• Loading from server.properties is a final catch-all that's attempted if no previous method of lookup has succeeded (#L1818).

We'd need a new logic flow:

1. Check the version file regardless of property name (this would catch CONSOLE_* properties and all other types).
2. Don't check server.properties if it's a CONSOLE_* property. (We could check server.properties too if we wanted. But that would be a separate consideration and a new feature to document).
3. Check server.properties for all other cases.

[donkers]

Hey, I was assuming that server_property() would need some work to support server properties in the version files, that is why I thought I would submit what I had so far. I will create a branch in my fork and begin work on supporting LOG_PATH as a version-able property there.

In the mean time I think even my current commits would be well received, as it adds support for all versions with minimal effort by server admins. People simply need to either set the correct DEFAULT_LOG_PATH for their preferred version in msm.conf or set msm-log-path= per server in the server.properties file.

[renderorange]

I'm okay with that, seems like a fair middle ground. At a minimum, I'd like to run through the install and testing with everything in your branch one more time, then I could accept the pull. If we all are in agreement, I'll create a feature request with the information from this pull request, just for documentation and another place to discuss, if the need arises.

[marcuswhybrow]

@renderorange & @donkers Agreed.

[renderorange]

tested again with centos 6.5 and ubuntu 14.04, both legacy and stable minecraft server versions.
I'm copying over our discussion notes from here to a new feature request for moving LOG_PATH to versioning files, as discussed.

[marcuswhybrow]

Thanks for the work @donkers. We trust successful pull requesters with write access. I've therefore added you to @msmhq/collaborators:

Less hurdles if you need to contribute in the future.