[Mono-dev] [PATCH] New eventlog implementation

[Gert Driesen]

> -----Original Message-----
> From: Atsushi Eno [mailto:atsushi at ximian.com]
> Sent: dinsdag 15 augustus 2006 14:15
> To: Gert Driesen
> Cc: mono-devel-list at ximian.com
> Subject: Re: [Mono-dev] [PATCH] New eventlog implementation
> 
> RunningOnLinux looks extraneous. It could just check the path validity
> and existence of "/var/blah" (besides it is not a precise name, as well
> as UnixEventLog which is very likely be used on Windows).

On your request, I've already changed its name to LocalFileEventLog locally.

> As for other comments I've already wrote to Gert in #mono.
> 
> In response to Gert's request, my final code review is done above.
> 
> My impression is, the patch is not worthy enough of being checked in,
> as it does almost nothing.

What do you mean "it does almost nothing" ? It handles eventlog creation and
registration, and does it in a way that exactly matches the MS
implementation. I know this is not a goal by itself, but I consider it to be
positive as it allow us to validate our implementation with unit tests (as I
did extensively in this patch).

I'm not sure I understand all this negativity. I created a good
implementation (which I've run by you first, before I started even working
on it), which is fully covered with unit tests, works seamlessly on both
Windows and Unix, allows us to achieve compatibility with MS.NET and still
allows us flexibility to use other implementations. What more do you want,
really ? 

> The purpose of the patch is to support Windows Event Log on windows.
> It is good, but the patch simply does not work. The code could be
> checked in later, when it became useful enough to read logs.

Can you explain what does not work ? All my unit tests pass 100% on Mono
(1.0 & 2.0 profile) and MS.NET (1.1 & 2.0).

As I've explained before, you need to:

- remove the event entries that were created by your implementation as it
does not write all necessary information and uses a different naming scheme
for the log files
- first create and event source before you can use it.

	For example:

		If (!EventLog.Exists ("enolog", "."))
			EventLog.CreateEventSource ("enosource", "enolog");

		....

> (If the purpose is just to support registry, it includes too many
> unnecessary changes. IF I took over the entire event log stuff, I'd
> just revert his changes to whatever I had.)

Nope, the purpose is not just to support registry. The purpose was to have
something useful that matches the MS implementation (allowing us to check
our implementation against the MS one using unit tests), and still allows us
to have flexibility to use other backend event log providers (such as the
local file implementation).

I don't really see what's wrong with that ? Would you care to explain ?

> Having said that, I don't object if someone approves the patch i.e. I
> don't want to spend my time anymore on this tiny demand on this
> registry based implementation.

Ok, thx.

Gert

> Gert Driesen wrote:
> > Hi,
> >
> > I've finished implementing the eventlog changes and incorporated
> > feedback by Atushi Eno (let me know if there's anything message).
> >
> > We now have the following environment variables for configuring the
> > eventlog subsystem:
> >
> > - MONO_EVENTLOG_TYPE for setting the implementation type.
> >
> >                 Possible value are:
> >
> >                 * win32 : read/write entries using the native win32
> >                 eventlog
> >                 * local : read/write entries as files to a local
> >                 directory
> >                 * null  : silently ignore all entries
> >
> >                 The default is "null", meaning entries will be
> silently
> >                 ignored.
> >
> >                 If the implementation is set to "local" and
> >                 MONO_EVENTLOG_PATH
> >                 is not set, then the following directory will be used
> >                 for storing eventlog entries:
> >
> >                 * windows : %APPDATA%\mono\eventlog
> >                 * unix    : /var/lib/mono/eventlog
> >
> >                 On unix, the directory permission for indivual
> eventlog
> >                 log directories is set to 777 (with +t bit) allowing
> >                 everyone to read and write eventlog entries while
> only
> >                 allowing entries to be deleted by the user(s) that
> >                 created them.
> >
> > - MONO_EVENTLOG_PATH for setting the local file path.
> >
> >                 If set, and MONO_EVENTLOG_IMPL is not set then the
> local
> >                 file implementation is used.
> >
> > - MONO_REGISTRY_MACHINE for setting the directory to use for storing
> > the machine-level registry hives.
> >
> >                 If not set, then the machine-level registry hives are
> >                 stored in directory .mono/registry relative to the
> >                 user's home directory.
> >
> >                 This applies to HKEY_LOCAL_MACHINE,
> HKEY_CURRENT_CONFIG,
> >                 HKEY_CLASSES_ROOT, HKEY_DYN_DATA,
> HKEY_PERFORMANCE_DATA
> >                 and HKEY_USERS.
> >
> > For this last environment variable, I'll supply a separate patch once
> > my other registry changes have been approved (to avoid a patch
> nightmare).
> >
> > A simple test application for the eventlog subsystem (by courtesy of
> > Atsushi Eno):
> >
> > using System;
> > using System.Diagnostics;
> >
> > public class Test
> > {
> >   public static void Main ()
> >   {
> >     if (!EventLog.Exists ("enolog")) {
> >       EventLog.CreateEventSource ("enologue", "enolog");
> >     }
> >
> >     EventLog log = new EventLog ("enolog", ".", "enologue");
> >     foreach (EventLogEntry e in log.Entries)
> >       Console.WriteLine (e.Message);
> >     log.WriteEntry ("My test message.");
> >   }
> > }
> >
> > Note: set the MONO_EVENTLOG_TYPE environment variable to "local" if
> > you want the entries to be persisted to the local file store.
> >
> > Let me know if there's anything that you want me to change, or if
> it's
> > ok to commit as such.
> >
> > I'll update the man page once my patch is approved (but before I
> > commit it).
> >
> > Best regards,
> >
> > Gert