Skip to content

Search upwards for package directory #2810

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

Closed
BardurArantsson opened this issue Sep 4, 2015 · 11 comments
Closed

Search upwards for package directory #2810

BardurArantsson opened this issue Sep 4, 2015 · 11 comments
Assignees

Comments

@BardurArantsson
Copy link
Collaborator

The idea here is that if the user's current working directory is somewhere inside a cabal package, then cabal-install should automatically find the .cabal file in one of the parent directories, instead of forcing the user to change to that directory.

@BardurArantsson
Copy link
Collaborator Author

Implementation issues/notes:

  1. Currently Cabal probably doesn't change the current directory to the .cabal file's directory (why would it?)
  2. The implementation (see pull request) is quite messy currently (quite a few FIXMEs) that need to be addressed. Most of them are simple (and I'll address them over the new few days), but a few are rather more complicated.

@hvr
Copy link
Member

hvr commented Sep 4, 2015

Fwiw, things like cabal test may benefit from temporarily changing the CWD as unit-tests which read external files assume to operate from the folder where the .cabal file is located. Maybe also TH code that reads external files has the same issue.

@BardurArantsson
Copy link
Collaborator Author

Yup, and the problem is... properly handling CWD has some potentially thorny issues:

  • We'd need to find all the places that require a change of CWD. It might be possible to find a single spot in cabal-install/Main.hs where this could be done -- I'm not sure if we have all the necessary location/configuration metadata in some common spot.
  • CWD handling on Windows is a royal pain because it's a per-drive in-addition to being a per-process thing. However, I guess we could pretty much assume we're always in a CWD on the same drive since we're doing a recursive upwards search, not crossing drives.
  • We'd have to reliably switch back the CWD to whatever it was before. The only really reliable way to do that is by spawning a new subprocess and then have that subprocess change the CWD and do all the work. This is the biggie -- it complicates things rather a lot and I'm not even sure we can do this sanely on Windows since it doesn't(?) support fork(). Either way this means that we'd have to make cabal-install a two-stage thing. We could try to do something with a bracket-like construct, but that would possibly be vulnerable to unforeseen signals and such OS-level interference.

A possible workaround would be to export an environment variable (like CABAL_SANDBOX_PACKAGE_PATH e.g. CABAL_PACKAGE_PATH) so that any code currently relying on CWD could at least be adjusted to look at that?

@BardurArantsson
Copy link
Collaborator Author

If we're going to do a forking type thing, an interesting idea might be to follow git's model of handling subcommands, namely just compiling each subcommand into a separate executable and then calling that from the main command. That would automatically "protect" the user's CWD from any interference by subcommands. What do you think about that idea?

(I realize that such a change definitely isn't trivial, but is it totally crazy?)

@hvr
Copy link
Member

hvr commented Sep 5, 2015

As I mentioned in #2349, why do we need to restore the CWD at all during cabal process' lifetime?

@BardurArantsson
Copy link
Collaborator Author

Hm. Now that I think about it... maybe we don't. I think I just did a brain fart.

@hvr
Copy link
Member

hvr commented Sep 5, 2015

so one less problem to worry about =)

I previously asked on #2811

Could you maybe outline the algorithm you use (or plan to use) for locating the various config-files?

I'd be interested in what you plan because I want to understand how the locations of sandbox-configs and .cabal configs etc are supposed to interact and relate to each other, as well as knowing if there's plans to emulate Git's GIT_DISCOVERY_ACROSS_FILESYSTEM logic or similiar.

@BardurArantsson
Copy link
Collaborator Author

Right, so right now the algorithm is literally what you see in getSandboxMetadata.

I think the most important thing here is to try to keep things as simple as possible. In particular there will be no magical merging of .cabal files, configurations or any such.

In the absence of sandbox-related command line flags, the only difference over current behavior should be that:

  • If cabal doesn't find a .cabal file in the current directory it just searches upwards and:
    • if it finds a .cabal file, it behaves as if it were running in that directory all along, i.e. we load that cabal file (and corresponding config, etc.) and set CWD before running any sub-command.
    • if it doesn't find a .cabal file, it complains as it used to do before if there was none in CWD

Now, the various flags complicate things a little, but the behavior I've implemented is this:

  • If a sandbox config file is explicitly specified on the command line (or via CABAL_SANDBOX_CONFIG) then there is absolutely no magical seach; Cabal just loads that configuration (or refuses to run further if the configuration doesn't exist -- I don't think searching here would be sensible because it could be dangerous to operate on a sandbox that the user didn't intend).
  • If nothing is specified on the command line, then we just behave as above.

I believe this is faithful to the way cabal-install behaves without my changes.

(The flag handling is the main reason that I combined the sandbox directory and config file paths into the SandboxMetadata data type -- without this unification the various control flows were getting absurdly hard to follow and were getting very deep. It was getting really hard to decide when to do a search, for instance.)

Does this make sense?

EDIT: Minor clarification

@BardurArantsson
Copy link
Collaborator Author

Regarding GIT_DISCOVERY_ACROSS_FILESYSTEM: I think it might be a useful thing to have, but I don't see it as particularly necessary since users typically don't have mulitple file systems in their home directories[1], and I don't see evil sysadmins putting .cabal files in /, or /home. (This would apply even if your $HOME is on NFS, I think.)

In the case of git, I think it would be more likely to have .git folders in various obscure directories for various sysadmin-related reasons and so the above reasoning wouldn't apply.

(My search code does cross filesystems, but I don't see it as particularly dangerous for the above reasons.)

[1] This may have changed with fuse, though. I guess it's worth considering defaulting to not crossing file system boundaries, though I don't know how easy it would be to do in Haskell. I'm guessing we'd need some platform-dependent code at the very least. (I'm guessing git uses device number comparison.)

@BardurArantsson
Copy link
Collaborator Author

This is turning into lots of fun...

I just discovered that replAction in Main.hs for some reason doesn't use the "standard" mechanism for loading the .cabal file. It just assumes it can use the CWD and look for it there. Sigh...

@BardurArantsson
Copy link
Collaborator Author

Closing since it's mostly redundant when we have nix-local-build. (And would have to be reworked entirely, anyway.)

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

2 participants