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

Relative path modules #71

Closed nyankers closed 2 years ago

nyankers commented 2 years ago

I'm not sure this is intended to be possible, but I've started using relative paths for modules so I can reuse the same .dgd file across (for example) different file systems, or different copies of the same project structure, and noticed there's a peculiar behavior.

The first time you run dgd with such relative paths, it runs them from the current working directory. However, if you hotboot dgd, then it seems to run them from the source directory. Given that the latter is how every other relative path in the .dgd file works, the post-hotboot behavior seems preferable.

This can be worked around by running the dgd executable from the source folder, which is what I've taken to doing, but since it's odd to me, I figured I'd report it.

I'm actually a little baffled how this is happening. I mean, it's clear that it's because it loads modules before it changes directories to the new source directory, but... I dunno why the relative source directory works if module loading doesn't!

As a total aside, besides that I'm even less sure if it counts as a bug or what, but while investigating this I found that the __DGD__ constant seems to be \x91 or possibly \x91\x09 (it's hard to be certain without getting gdb out and checking, since it's not a string but a literal sequence of characters that will fail compilation, and I'm not sure of anything that relies upon this anyway) rather than what the comment in ppcontrol.cpp suggests (HT 1 HT).

dworkin commented 2 years ago

I'm not sure this is intended to be possible, but I've started using relative paths for modules so I can reuse the same .dgd file across (for example) different file systems, or different copies of the same project structure, and noticed there's a peculiar behavior.

The first time you run dgd with such relative paths, it runs them from the current working directory. However, if you hotboot dgd, then it seems to run them from the source directory. Given that the latter is how every other relative path in the .dgd file works, the post-hotboot behavior seems preferable.

This is a good point. The current behaviour is intentional, mostly due to the fact that originally, the only way to load a module was through a command-line argument (-e), which is still supported and which definitely should not be relative to the source directory. But there is no reason why the modules specified in the configuration file should not be relative to the source directory.

This can be worked around by running the dgd executable from the source folder, which is what I've taken to doing, but since it's odd to me, I figured I'd report it.

There are some dangers to running DGD from the source directory. Don't put the binary, config file or swapfile in there, or they can be read/modified from LPC code.

I'm actually a little baffled how this is happening. I mean, it's clear that it's because it loads modules before it changes directories to the new source directory, but... I dunno why the relative source directory works if module loading doesn't!

That is odd. Are you sure the same configuration file is loaded in both cases?

As a total aside, besides that I'm even less sure if it counts as a bug or what, but while investigating this I found that the __DGD__ constant seems to be \x91 or possibly \x91\x09 (it's hard to be certain without getting gdb out and checking, since it's not a string but a literal sequence of characters that will fail compilation, and I'm not sure of anything that relies upon this anyway) rather than what the comment in ppcontrol.cpp suggests (HT 1 HT).

That bug dates back to 2010! I fixed it early this morning.

I'll take a look later today at the relative path for modules issue.

nyankers commented 2 years ago

This can be worked around by running the dgd executable from the source folder, which is what I've taken to doing, but since it's odd to me, I figured I'd report it.

There are some dangers to running DGD from the source directory. Don't put the binary, config file or swapfile in there, or they can be read/modified from LPC code.

To clarify, I mean doing cd src before calling the dgd binary via something like ../../dgd/bin/dgd. Lots of ..s this way. I don't mean they're actually in the source directory. ^^

I'm actually a little baffled how this is happening. I mean, it's clear that it's because it loads modules before it changes directories to the new source directory, but... I dunno why the relative source directory works if module loading doesn't!

That is odd. Are you sure the same configuration file is loaded in both cases?

Oh. Actually I made an even dumber mistake. I'd moved all these to relative paths a while ago, and mistakenly thought I'd already swapped the source directory in advance of that, but checking my backups, I did the source directory and the modules all on the same day.

Testing a scenario where the modules use absolute paths and the source directory uses any relative path other than ./ (or an equivalent) seems to fail on hotboot (Config error: bad base directory "./src"), as seems sane (if not particularly desirable) since they both access the paths in approximately the same way.

So there's no difference in behavior between modules and the source directory. They both expect a relative path based on your $PWD when first running dgd, which is fair. However, after hotboot, they now expect a relative path based on the source directory.

The workaround is the same, of course, to cd into the source directory from the start, and make all relative paths based on that.

dworkin commented 2 years ago

Modules are loaded after the directory is changed, as of 46ca615406738545877b9bbee5bb9b78aed6e0ed.

nyankers commented 2 years ago

This fixes the modules inconsistency, though the same issue is there for the source folder (as stated above, the only relative path that works after hotbooting for directory is something like ./).

The only easy solution I can think of is maybe storing the current working directory somewhere before changing directories to the source folder, so we can switch back before hotbooting.

Another albeit incomplete solution is to avoid changing directories at all after hotbooting (with the presumption that since we've hotbooted, we're already in the source directory), though this does mean that you wouldn't be able to change source paths when hotbooting.

Of course, the workaround of using ./ works just fine, so it's not like it's a problem per se. You could even change the source directory by updating the .dgd file for just a single hotboot, then switching back to ./, if you needed to. ^^;

dworkin commented 2 years ago

This fixes the modules inconsistency, though the same issue is there for the source folder (as stated above, the only relative path that works after hotbooting for directory is something like ./).

Just . should do fine. directory is the one thing that the others are relative to, and I'm okay with that being the exception.

The only easy solution I can think of is maybe storing the current working directory somewhere before changing directories to the source folder, so we can switch back before hotbooting.

You call it easy, I call it silly.

Another albeit incomplete solution is to avoid changing directories at all after hotbooting (with the presumption that since we've hotbooted, we're already in the source directory), though this does mean that you wouldn't be able to change source paths when hotbooting.

Of course, the workaround of using ./ works just fine, so it's not like it's a problem per se. You could even change the source directory by updating the .dgd file for just a single hotboot, then switching back to ./, if you needed to. ^^;

Or just use distinct pre-hotboot and post-hotboot configuration files.

Without thinking too deeply about whether this is a good idea, note that the preprocessor works in the config file. I use this for shared but slightly distinct configuration between DGD and Hydra, with #ifdef __HYDRA__. Now if there were a preprocessor symbol like __HOTBOOT__... No, I don't like this much.

nyankers commented 2 years ago

Rather than hardcoding __HOTBOOT_, which does feel rather specific and contrived, if you wanted to capitalize the config preprocessor and make it more useful, you could do something like what a number of languages do (java, c/++, heck dgd's makefile does it a bit, and especially if you want to turn on optional features!) and allow the user to pass in definitions on the command line.

e.g. something like dgd -DHOTBOOT blah.dgd or dgd -DSOURCE="." blah.dgd would respectively define HOTBOOT for an #ifdef HOTBOOT and define SOURCE for something like

#ifdef SOURCE
directory = SOURCE
#else
directory = "/home/dgd/src"
#endif

And these definitions could be passed along to the LPC via something like a /include/env.h type header file, for example a user could pass in a DEBUG definition that their LPC code checks to determine how verbose and noisy errors should be.

Obviously none of this is impossible to arrange now, so it's all definitely a "nice-to-have" thing.

dworkin commented 2 years ago

That would be in line with what everyone else does, along with overrides through environment variables, etc. etc. I hate it all.

I want all of the configuration to happen in the configuration file. Now there may be a case for passing on settings to LPC, through an optional defines config file parameter. Though that can already be done with an alternate include_file.

nyankers commented 2 years ago

I'm actually a bit curious what the purpose of the preprocessor is within the config file. ^^;

Besides __DGD__ and __HYDRA__ mentioned above, of course, which seems just as valid of a scenario to have multiple files for as anything else. I suppose there's also some limited degree of reducing duplication, but without basic LPC compilation, there's a lot of limitations there (no string concatenation, no building lists, etc.)

I'm also curious why #include doesn't work in it, which leads me to a secondary thought. Right now, a relative source path is from the perspective of the current working directory (which changes post-hotboot, causing this gotcha), but it could work as the default #include logic would, that is from the perspective of the config file.

Anyway, although it's a little bit funny having so many ..s, my current config works just fine, so if you think that's a fine limitation for relative source paths, then this can probably be closed. I'm just commenting at this point. ^^;

dworkin commented 2 years ago

I'm actually a bit curious what the purpose of the preprocessor is within the config file. ^^;

Besides __DGD__ and __HYDRA__ mentioned above, of course, which seems just as valid of a scenario to have multiple files for as anything else. I suppose there's also some limited degree of reducing duplication, but without basic LPC compilation, there's a lot of limitations there (no string concatenation, no building lists, etc.)

It is mostly about those predefined symbols, plus defining and using macros in the config file. It is definitely limited, intentionally so, since a full preprocessor environment can only be initialized after the config file is read.