dworkin / dgd

Dworkin's Game Driver, an object-oriented database management system originally used to run MUDs.
https://www.dworkin.nl/dgd/
GNU Affero General Public License v3.0
103 stars 31 forks source link

Suggestion: make `directory` setting in config file relative to location of config file, rather than relative to cwd at invocation of DGD #73

Closed shentino closed 2 years ago

shentino commented 2 years ago

I saw some recent commentary about module arguments, and I got to thinking about relative paths.

I think the config file's paths should be relative to the config file itself, rather than wherever you happen to be executing DGD from.

At present, particularly during a hotboot, the config file is interpreted relative to the mud directory. Which, if not the same directory as where you first executed DGD, has some complications.

Issue #71 is related. I was tempted to comment there but it's closed and I'm not sure if this is completely the same issue or not.

However, it may bring some consistency if DGD interpreted paths in the config file relative to the location of the config file itself.

Since it was mentioned that the preprocessor handles the config file it also reminded me that include files within the mud directory are themselves interpreted relative to the location of the including file itself during LPC compilations, for example an #include "../foo.h", and I think it may be more intuitive if the config file followed suit.

It would also be simpler than having two separate config files, one pre hotboot and one post hotboot.

Of note at present kotaka handles this by having the hotboot parameter specify a small shell script that does some checking on the snapshot files before directly exec'ing a proper command line for DGD, from the same directory as the config file itself, which is stored in the top level of the repo, with the mud directory a neighboring subdirectory, and which is where my monitor script originally invokes dgd. I use a single config file for both, and I would prefer to keep this as is because having two separate config files for pre and post hotboot is at best redundant and at worst may lead to inconsistent configurations.

My suggested implementation is to have DGD cd to the config file's containing directory before interpreting any paths within, including but not limited to the mud/source directory as well as any module arguments.

dworkin commented 2 years ago

I fail to see the reason for making the path to the auto object, or the standard include file, or various other paths relative to the config file, which would make it possible to have them outside of the mud directory. To me, that is just nonsense. I will assume for the sake of argument that you only want to make the following paths relative to the config file: directory, ed_tmpfile, swap_file, dump_file, modules.

DGD changes directory to the mudlib directory before interpreting any other paths in the config file. What you are asking for is that DGD maintains two simultaneous notions of "current directory": one for the directory that contains the LPC objects, and one for the other paths mentioned above, where DGD must also be able to open files at any time beyond the configuration stage.

What you are trying to fix is a limitation of the unix file model (a program can only have one current directory at a time). Moreover, there already is a solution, and that is absolute paths.

My position is that the current situation, paths relative to the mudlib directory plus absolute paths for exceptions, is already satisfactory. You'll have to convince me that that is not the case.

shentino commented 2 years ago

My apologies, it seems I wasn't clear in what I meant and that led to a misunderstanding on your part. And as understood I completely agree that it would be nonsense had I meant it how you saw it.

The only directory I wanted to be interpreted relative to the location of the config file was directory, i.e., the mudlib directory. The one directory that DGD maintains as current directory. Why I chose to use the plural form of "paths" is beyond me and I suspect it was a sloppy mistake in phrasing on my part.

As it is I can see that not even ed_tmpfile, swap_file, dump_file, and modules were intended to be affected, only directory so I definitely misphrased.

The sequence I expected to be followed were this suggestion implemented is as follows:

  1. DGD, invoked from the command line or a shell script, locates the config file
  2. The config file is read
  3. The directory argument is read and interpreted relative to the directory containing the config file as the mudlib directory
  4. DGD chdir's to the mudlib directory
  5. All paths henceforth are interpreted relative to the mudlib directory, including the remaining paths in the config file, EXCEPT for directory in the event of a hotboot which is just an alternate version of step 1

One of the sticking points that was bothering me about the status quo was that the interpretation of directory depended on the current directory in the shell environment from which DGD itself was invoked, and that you could change its meaning simply by invoking DGD from any arbitrary location. Given that the config file itself is static, it appears that a relative path for the "anchor" setting of directory can be changed arbitrarily simply by changing where DGD is invoked from.

It makes more sense to me to interpret directory relative to the config file itself, in the same way that #include directives are interpreted within LPC when themselves present in an include file.

Relatedly, but somewhat separate, is that directory, being currently interpreted relative to the location of DGD's invocation, would thusly at present be interpreted relative to itself in the event of a hotboot. If the config file is reused, this would make . the only viable value to avoid DGD chdir'ing somewhere else during a hotboot. Not reusing the config file as mentioned leads to redundancy. Also, unless DGD's original invocation is itself made from inside the mudlib directory, DGD's interpretation of directory during initial boot will conflict with said interpretation during a hotboot.

I'm not trying to cause DGD to maintain two "current directories" as I mistakenly led you to assume, only one. Just to interpret directory in the context of where the config file itself is located rather than the likely arbitrary context of wherever you happen to be invoking DGD from in the shell, which gets complicated by a potentially conflicting context of hotbooting, and the conflict which can only be at present resolved either with an absolute path in directory or having two separate config files seems to hamper the flexibility of the recent feature of allowing a relative path in directory in the first place

shentino commented 2 years ago

So yes the only change I wanted was for interpreting the location of the mudlib directory itself, as relative to the location of the config file rather than whatever arbitrary location you invoked DGD from, which may even contradict the hotboot case.

dworkin commented 2 years ago

Relatedly, but somewhat separate, is that directory, being currently interpreted relative to the location of DGD's invocation, would thusly at present be interpreted relative to itself in the event of a hotboot. If the config file is reused, this would make . the only viable value to avoid DGD chdir'ing somewhere else during a hotboot. Not reusing the config file as mentioned leads to redundancy. Also, unless DGD's original invocation is itself made from inside the mudlib directory, DGD's interpretation of directory during initial boot will conflict with said interpretation during a hotboot.

What exactly is the problem with this? If you're using a startup script for DGD -- as I assume you are already doing -- that script can chdir to the mudib directory and execute DGD in precisely the manner specified by the hotboot parameter in the config file. What is the issue with having . as the mudlib directory?

shentino commented 2 years ago

DGD could also be invoked from the command line directly, and in fact sometimes I do so locally during testing

Most of the time I invoke DGD from the top level of my mud's git repo, the same location from which I invoke the startup script, and where the stderr redirect, the state and tmp directories, amd the mudlib directory itself are contained

Using . as the directory and chdir ING there at launch time is a workaround but it kind of defeats some of the flexibility of allowing a relative path in the first place

My main point is consistency of context for interpreting directory, both with and without hotboots, that doesn't depend on where the shell is or whether you're invoking DGD directly by command line from God knows where or from a script that may already have a well defined cwd

This is minor but I prefer to stay out of the mudlib directory entirely when not in DGD. I see the mudlib directory as intended akin to a chroot jail and it seems cleaner to confine any external work to the top level containing the lpc directory

dworkin commented 2 years ago

DGD could also be invoked from the command line directly, and in fact sometimes I do so locally during testing

It's entirely reasonable to have a different config file for testing.

Most of the time I invoke DGD from the top level of my mud's git repo, the same location from which I invoke the startup script, and where the stderr redirect, the state and tmp directories, amd the mudlib directory itself are contained

In that case the startup script will also be handily available, and you could just use that?

Using . as the directory and chdir ING there at launch time is a workaround but it kind of defeats some of the flexibility of allowing a relative path in the first place

How exactly does . for directory defeat the flexibility? You seem to be arguing, rather, that it is too flexible.

My main point is consistency of context for interpreting directory, both with and without hotboots, that doesn't depend on where the shell is or whether you're invoking DGD directly by command line from God knows where or from a script that may already have a well defined cwd

Use an absolute path for directory.

shentino commented 2 years ago

It isn't about flexibility, but consistency.

I see the config file as a static, immutable file the interpretation of which should not be influenced by external factors such as the shell s cwd

It gives a variable meaning to a constant value

shentino commented 2 years ago

I should probably ask before I assume, why was 'directory' made relative in the first place?

dworkin commented 2 years ago

I should probably ask before I assume, why was 'directory' made relative in the first place?

You made it relative. Enabling that was requested in this issue.

nyankers commented 2 years ago

PR #31 is the relevant discussion, I believe. You were there and even noted this at the time.

Then I noted that the only relative path that works across hotboots is . in #71, because I'd finally swapped to relative paths, ironically specifically to make sure my test environments and my actual server do not use different config files. ^^;

shentino commented 2 years ago

I should probably ask before I assume, why was 'directory' made relative in the first place?

You made it relative. Enabling that was requested in this issue.

I made a mistake in presentation. I'll clarify later but for now I owe you an apology for being unprofessional

shentino commented 2 years ago

First of all, I owe you an apology for having an arrogant attitude in this issue tracker, trying to insist rather than convince diplomatically. I'm fortunate this isn't a professional programming environment because it's likely I'd be risking a formal reprimand for insubordination.

Secondly, I had completely forgotten that the original suggestion was mine in the first place. Which, again, I owe you an apology for.

Thirdly, related to the second point, I now realize that I probably naively made assumptions about how that feature was going to be implemented, and the arguments I'm making now after the fact would have been better suited for the original feature request, and I should have been more specific in #32 regarding the context in which said relative path was going to be interpreted in the first place.

Had I known then what I noticed now, I would have tailored my suggestion to take into account the config file's actual location.

I'm also wondering if I should do more than just make a suggestion, and I find myself curious if my case would be better made by some work coming up with the actual patch as Noah seems to have done.

PR 31 also doesn't seem to have addressed the hotboot case, and the contradiction between noah's usage case and hotboots is one of the main reasons I advocated for changing the context from which directory was interpreted.

But overall I think my primary fault is being sloppy with my original feature request #32 by not being specific enough, followed by my arrogant attitude in this one.

I apologize for both.

dworkin commented 2 years ago

Given that you raised #32 "for documentation", I believe that it has served that purpose.

All apologies accepted. Let me lay out my case against interpreting directory in a special way:

nyankers commented 2 years ago

Considering your criticism is that using the config file's directory would be "unintuitive" and "purely technical," I have to ask what's intuitive and not purely technical about the current approach?

dworkin commented 2 years ago

Current working directory, relative and absolute paths: these are well-established concepts which I consider intuitive. The only exceptional situation is hotbooting, where the difference between previous and current working directory comes into play.

Taking one path relative to another path, which is not a directory: this is what I consider unintuitive. It simplifies the afore-mentioned special case, but otherwise introduces only confusion -- move a config file from one place to another, and it might stop working.

I hope that clarifies things.

shentino commented 2 years ago

For me hotbooting adds an extra angle to the issue that by itself might not be that serious, but when combined with noah's original issue is when it seems to me to advocate for a more general solution that handles both cases at the same time.

Hotbooting isn't just an exceptional situation, it's one whose present solution conflicts with the status quo.

That's my main sticking point, is that my idea would reconcile both issues together. Honestly it feels like the two cases presently have conflicting needs. It isn't just about handling hotbooting, but doing so in a way that doesn't also mess up noah's case.

As a matter of perspective, every time I edit the config file, I keep staring at the mudlib directory next door, and I'm always seeing it from the point of view of the config file, as "mud" or "./mud", and having "." as the mudlib directory because that's where DGD is going to be parking its cwd after a hotboot is definitely unintuitive to me, especially if I'm executing the warm or cold boot of DGD from the directory holding the config file. Having "." as the only "legal" value that would reconcile both cases seems to rob directory of some of the potential flexibility that allowing a relative path would provide.

And IMO, directory is itself already a special case anyway because it is the root from which the other paths are interpreted after DGD chdir's to it, whereas directory itself is interpreted relative to cwd at launch before the chdir occurs. Setting it to . just makes the chdir moot by requiring you to chdir into the mudlib directory before launch (or hotboot).

Your suggestion to use separate config files for pre and post hotboot would take care of the issue but it has the aforementioned problems of duplicated configuration information. Though on that note, if you could use #include directives of a sort in the config file, that would greatly mitigate the redundancy concern. For such I would be happy to have the top level config files only specify the required directory setting, and have both of them simply #include the common base settings such as users and editors and so on. Might be an interesting idea to consider in the future.

Overall I still don't agree, but at this point I am willing to concede that it is at least a matter of opinion. As such it's not a complete disaster if the status quo is preserved.

You have clarified things to where I can at least get a better grasp on your point of view even if I don't agree with it, so that's appreciated.

nyankers commented 2 years ago

I think there's another point to be made for consistency that DGD's "default" behavior for relative files in LPC code is to base the relative path from the code file. Given that the config file is partially processed like an LPC file, you could make the argument that it should work as any other LPC file does.

Of course, although there is default behavior for inherit and #include, I don't think DGD permits that to ever actually occur, instead throwing an error if the driver does not handle these explicitly, and as the driver is very limited on what it can inherit, the only case you get to know how DGD canonically handles relative file paths within LPC is if the driver itself includes a file using one.

You could argue that the config file is not actually an LPC file, or that it is not actually a program, or any other number of such things, but I think it is not a very strong argument in favor of intuitiveness to say that two things that look alike should behave differently.

This is definitely a subjective topic though. I mean, if you wanted my personal take, I'd actually argue what @dworkin brought up earlier, that all file paths should be relative to the config file, except those that have to do with the virtual file structure within LPC itself (include_file, include_dirs, auto_object, and driver_object), and at least the latter of the two should not be allowed to be relative anyway. I'm not sure about how include_file works to comment on that one, but include_dirs seems to me like it's really just a bunch of string prefixes to stick before a #include <foo> preprocessor instruction, so they aren't actually file paths at all even if most people would use them as such.

I'd also argue that the config file should be king to the point that the user should not even have to provide a snapshot file to read, because the config file already defines the snapshot file.

This way, if someone zips up a DGD code base that includes a config file, source code, snapshots, and virtually everything else, and sends it off to someone else, that other person only needs to unzip it and run a compatible version of DGD pointing at that project's config file. Of course, things get messier when compiled modules get involved, you might need a makefile or something for that.

However, I'm not going to hack any of this to work that way without @dworkin 's blessing, because whatever scripts I have to write to make things work now are nevertheless far simpler than the .cpp changes I'd have to make to the source code and then maintain.

shentino commented 2 years ago

I mostly agree, except that the non LPC directories/files (the snapshot, swap, and ed files) are already being opened relative to the mudlib directory and changing them at this point would break backward compatibility with existing config files, which isn't exactly a showstopper but would be a bit annoying.

As for the paths you mentioned they are interpreted in the context of the LPC environment itself and are completely internal to the LPC programming environment. Not only are they relative to the mudlib directory but DGD turns it into what basically amounts to a chroot jail, and thus they can't be interpreted absolutely, at least not relative to the actual filesystem root hosting DGD itself.

For the hotboot case at present I'm actually using a full blown shell script as a stub stand-in for DGD itself, and its sole purpose is to audit the snapshot files present and then exec the new DGD with the appropriate command line arguments.

In the process I also noticed I had to do a cd .. to park the script at the same directory the monitor script invoked DGD from during initial launch (for either cold or warm boot depending on what snapshot files the monitor script finds).

The tricky part is that the script is also in the interim floating any fd's that the former DGD left open associated with connected users and lets the exec'ed DGD inherit them :P. It feels a bit hackish and I was mildly surprised it worked without the fds being munged somehow on the way through the script.

I have to admit hotbooting into an interim shell script instead of directly into DGD feels a bit unorthodox as a usage of the hotboot parameter :P.

nyankers commented 2 years ago

They're executed relative to the source directory, but within the context of LPC, they're absolute paths.

For comparison, in DGD, you might do something like #include "include/header.h" which, even within the context of LPC, would be a relative path (unless your driver does very funky stuff).

nyankers commented 2 years ago

I've tried hotbooting into a script in the past, but it never quite worked.

Honestly, the only thing my own execution script really does is handle a few paperwork-like things DGD seems to intentionally leave up to the user to handle:

  1. CDing into the source directory so that the relative . path work across hotboots
  2. Appending arguments based on the existence or lack thereof of snapshot files.
  3. Handle logging standard output.

If I ever needed a complex config file that varies by environment, I'd be adding "making sure the config file is up-to-date" to the list.

shentino commented 2 years ago

Take a look at https://github.com/shentino/kotaka/blob/master/script/hotboot for an example

The "monitor" script in the same directory is a manager script that handles the actual invocation of DGD, which itself exec's the hotboot script

I use two separate scripts at the same time

shentino commented 2 years ago

They're executed relative to the source directory, but within the context of LPC, they're absolute paths.

For comparison, in DGD, you might do something like #include "include/header.h" which, even within the context of LPC, would be a relative path (unless your driver does very funky stuff).

My apologies btw, I was meaning to clarify an apparent discrepancy, I didn't know you already meant that.

I also wanted to emphasize for any readers.

shentino commented 2 years ago

For clarity and as an exercise in follow-through I amended the title of this issue.