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

Problems with interaction between config files and --seek-file/--file #452

Open
levic opened this issue Mar 23, 2023 · 2 comments
Open

Problems with interaction between config files and --seek-file/--file #452

levic opened this issue Mar 23, 2023 · 2 comments

Comments

@levic
Copy link

levic commented Mar 23, 2023

(macOS, python3.11, doit 0.36.0)

Use case I'd like to be able to invoke doit from anywhere in a project tree.

For brevity, when I write --seek-file/--file I'm also including the environment variables DOIT_SEEK_FILE/DOIT_FILE

Example:

  • I'm in mymodule/foo/bar and I want to lint one specific file
  • doit lint myfile.py is much better developer experience than cd ../../.. ; doit mymodule/foo/bar myfile.py ; cd mymodule/foo/bar

(Related: #222)

Problem/Bug: Despite specifying --seek-file/--file config files will only be loaded from the current directory, and not every config option can be specified in a dodo.py DOIT_CONFIG

Details

[1a] Config files are initially checked in DoitMain.__init__() using the current directory. This takes place before --seek-file/--file have any effect. This means that only config files in the current directory work.

[1b] If there are multiple pyproject.toml files in your directory tree then you can get different behaviour depending on where you invoke doit.

An example: You cd into a virtualenv to examine an installed package's code. This package has its own pyproject.toml with doit configuration. When you run doit the that package's directory then its doit config will be used -- even though you have specified --file.

While this is an expected risk if using --seek-file, as a developer this is surprising and unexpected behaviour for --file. I would expect that specifying --file would mean only my configuration is used.

[2] We need the ability to specify a config file because not all options work with DOIT_CONFIG

Consider cwdPath:

  • cwdPath setting is evaluated in get_module() and os.chdir() is called
  • Later, dodo.py's DOIT_CONFIG is read by the loader (in NamespaceTaskLoader.load_doit_config()). This is after get_module() has already been called. Specifying cwdPath in DOIT_CONFIG has no effect.

One way to work around this is to wrap every task with a decorator that inserts a new action to change the current directory, but that would have to be done to every task.

A better way of handling it would be to create a custom loader. We could subclass DodoTaskLoader and override load_doit_config() to change the current directory if cwdPath is set in DOIT_CONFIG. The problem is that a custom Loader can only be specified through the config file because plugins are loaded before DOIT_CONFIG is read.

Since the loader is responsible for loading DOIT_CONFIG we have a circular dependency.

Possible Solutions

Option 1:

Add a new --config-file/DOIT_CONFIG_FILE option & environment variable.

  • This is relatively easy to implement and understand
  • This doesn't fix the unexpected loading of config files in the current directory when using --file or --seek-file
  • Questions/Edge Cases:
    • If --config-file is specified and there is no dodoFile setting then do we look for dodo.py in the current directory, or do we look for it in the same directory as --config-file?
      • If we look for it in the same directory as --config-file then do we really need a separate option? We could use --file for both.
      • If we use the current directory (or a parent directory --seek-file) then this would allow for multiple dodo.py files with a single shared config file, but this seems like an obscure usage pattern.

Option 2:

If --seek-file/--file is specified, the config file will always be assumed to be in the same directory as dodo.py

  • This would change the current behaviour, but if you have a config file then the current behaviour is arguably broken right now anyway (see [1b] above)
  • If you are just using --seek-file/--file with no config files then there would be no change (unless you are using pyproject.toml for non-doit purposes and in a directory below your top-level dodo.py)
  • Questions/Edge Cases:
    • What if we find a config file without a dodo.py? Should --seek-file stop searching? What if the config file specifies dodoFile?
    • What if we find a doit.cfg and a dodo.py but the doit.cfg specifies a dodoFile that's not dodo.py? (If I read the code correctly, then right now the config file wins)
    • What if a pyproject.toml is found without a dodo.py and the pyproject.toml has no doit-related entries? Do we keep searching or raise an error that no dodo.py is not found?
  • Option 2 Proposed Solution
    • --seek-file should immediately stop when it finds any of pyproject.toml, dodo.py or doit.cfg and set the current working directory to that directory (even if pyproject.toml doesn't contain any doit settings).
    • If --file is specified then:
      • If the file ends with .cfg or .toml we treat it as a config file. If the config file doesn't specify a dodoFile then it defaults to a dodo.py in the config file's directory.
      • If the file ends with .py then we treat it as the python task file. If there's a config file in that directory then we load it and if we find a dodoFile setting we ignore it or raise an error
      • If the file doesn't end in a known extension then we raise an error
      • The rationale here is that the command-line argument should always be given the highest priority.
    • If both --file and --seek-file (or their environment variables) are specified then an error should be raised.
      • This might be a little surprising if the user specifies eg --file+DO_SEEK_FILE or DO_FILE+--seek-file (one from a command line argument and one from an env var), but it avoids any ambiguity and is easier to implement as we don't have to keep track of whether an argument came from an env var or the command line.

If @schettino72 is happy with Option 2 then I'm willing to take a look at implementing it.

It's not as simple a change as I initially thought because the config file loading takes place before the command line argument parsing (and the results are used to set the command line argument defaults). To preserve this behaviour would mean refactoring to load the config file options twice -- once before the command line arguments are parsed and once afterwards.

Fund with Polar
@levic levic changed the title Some config options don't work with --seek-file/DOIT_SEEK_FILE/--file/DOIT_FILE Problems with interaction between config files and --seek-file/--file Mar 23, 2023
@rix0rrr
Copy link

rix0rrr commented Nov 23, 2023

I'm running into this at well.

Wouldn't it be sufficient/nicer to search upwards for the dodo.py file, from the current directory?

@schettino72
Copy link
Member

I think that config files (pyrpoject.toml or doit.cfg) should always be on the same path as dodo.py.

Would that solve the problem? That should be easy to fix...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants