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

Supporting builddir in cabal.project and global config #5271

Open
merijn opened this issue Apr 18, 2018 · 19 comments
Open

Supporting builddir in cabal.project and global config #5271

merijn opened this issue Apr 18, 2018 · 19 comments

Comments

@merijn
Copy link
Collaborator

merijn commented Apr 18, 2018

To the best of my knowledge, new-X don't currently obey builddir entries in cabal.project. There was some speculation that there was a technical reason for this, but at first glance of the code it just seems the problem is laziness. That is, all code paths dealing with computing the DistDirLayout try and resolve the cabal.project path and builddir in a single step.

It seems fairly straightforward* to split this into two steps, first looking up cabal.project and the project path, then checking establishing the DistDirLayout after that. I would assume that the logical priority for determining builddir would be:

  1. commandline --builddir flag
  2. builddir in cabal.project
  3. default to dist-newstyle

If no one has any theoretical objections I'll try and see if I can get it working as straightforwardly as I hope it to be :p

[*] - Famous last words.

@alexbiehl
Copy link
Member

I think it won't be that easy. There is a knot:

  • Parsing of configuration (basically rebuildProjectConfig) uses file monitors to check for changes in config files
  • File monitors use a cache to be able to tell file changes over multiple runs of cabal
  • These caches have to be stored somewhere... and of course its the dist directory

So rebuildProjectConfig, the function that reads and assembles our project configuration, actually writes to dist dir on a cold start. Not sure how you would work around that.

@alexbiehl
Copy link
Member

Maybe we should bite the bullet and stop caching of ProjectConfig stuff?

@merijn
Copy link
Collaborator Author

merijn commented Apr 18, 2018

Well, we can just parse the cabal.project, check what the builddir is, THEN check with the cache if anything changed and things need to be rerun. Sure, that introduces a redundant read/parse of cabal.project if the config hasn't changed, but I doubt reading and parsing cabal.project is so slow that it will noticeably impact the speed of any cabal command.

@hvr
Copy link
Member

hvr commented Apr 18, 2018

@merijn the problem is that you're assuming that we've already fully implemented all features; but there's more planned, like file-globbing includes for cabal project files. And then I'm afraid the induced latency would be a lot more noticeable, and it'd also risk destroying the latency savings that caching would give us (I care to keep the no-op detection latency way below 100ms, as otherwise cabal new-run becomes useless for my scripting)

@merijn
Copy link
Collaborator Author

merijn commented Apr 18, 2018

I think the ability to specify a builddir loses most of it's value if you can't do it per-project and you have to always specify it on the commandline for each command. At that point, why even support specifying a custom builddir at all?

@23Skidoo
Copy link
Member

23Skidoo commented Jun 8, 2018

Looks like it's not certain we actually want to implement this, so labeled as "discussion".

@AndreasPK
Copy link
Collaborator

Maybe we should bite the bullet and stop caching of ProjectConfig stuff?

I've also had issues with the stored (outdated) config overwriting command line settings for store-dir.
Might also also be a result of the cache not being recognized properly as outdated.

I have not opened a Ticket on that yet as it's not quite clear to me when that happens.

@hvr
Copy link
Member

hvr commented Aug 1, 2018

I had a brief discussion with @alexbiehl for two potential pragmatic workarounds; either,

  • Introduce a new file cabal.builddir which is located the same way cabal.project is, but merely contains a string denoting the --builddir argument value; this is basically a textual symlink (Git uses something like this too inside its .git folder), or

  • Allow builddir: ... inside cabal.project files but in a restricted way that can be scanned for by a trivial dumb parser, and w/o needing to read the full cabal.project file; this should allow to keep the latency overhead well in the 1ms order of magnitude.

The latter approach could be implemented as follows:

The key/value pair

builddir: <path>

  • must occur (completely, i.e. including a terminating LF) within the first 4096 bytes;
  • it must start in column one, and
  • must be a single line, and it
  • must be lower-case and there
  • must be no whitespace between builddir and :
  • there must at most be a single occurrence of builddir (as we only scan for the first one)

we can validate this easily when doing a full parse of the cabal.project by doing a consistency check (the full parser must report the same builddir as the lightweight parser; this is a similiar scheme to the one we already used for the cabal-version scanning)

Then scanning is very cheap, assuming that projfrag contains the prefix of a cabal.project file:

  • Try stripPrefix "builddir:" projfrag, if successful take rest of line
  • Otherwise, breakSubstring "\nbuilddir:" projfrag, if found, take rest of the line after the :
  • Otherwise, there was no (valid) --builddir

@23Skidoo
Copy link
Member

23Skidoo commented Aug 1, 2018

I lean towards the second solution.

@phadej
Copy link
Collaborator

phadej commented Dec 12, 2019

The second approach feels very restrictive. I can imagine someone asking for builddir inside if conditionals after/if those are added to cabal.project, i.e.

if impl(ghc ==8.0.*)
  builddir: dist-ghc-8.0

why one would like to do that? So one can access plan.json for example. Making builddir special would ruin that.


Parsing cabal.project should really be a bottleneck, Using parsec parser (though very simple on, in cabal-install-parsers, the small cabal.project is parsed in

benchmarking haskell-ci.project
time                 22.82 μs   (22.27 μs .. 23.26 μs)
                     0.998 R²   (0.997 R² .. 0.999 R²)
mean                 22.20 μs   (22.06 μs .. 22.43 μs)
std dev              624.3 ns   (376.6 ns .. 852.1 ns)
variance introduced by outliers: 30% (moderately inflated)

haskell-ci.project:

-- Cabal project configuration
-- Consult http://cabal.readthedocs.io/en/latest/nix-local-build.html for more information

packages: .
packages: cabal-parsers

tests: true

package haskell-ci
  ghc-options: -Wall
  ghc-options: -Werror

And if that will become a problem, we can cache the parsed Project, there's FileMonitor stuff which does exactly that. (EDIT: and rather figure out where to cache the parsed cabal.project, as builddir isn't yet known)

@szabi
Copy link

szabi commented May 26, 2020

As #6849 was closed as duplicate of this I'd like to bring in some points mentioned there.

  • The documentation of the flag only talks about cabal.project-files. I took this as the local cabal.project files. It should be somewhere mentioned that this equally applies to ~/.cabal/config -- that wasn't immediately clear to me as a user.

@phadej
Copy link
Collaborator

phadej commented May 26, 2020

The core problem is that --builddir determines the cache location. Reading configs is noticeable time-sink which makes cabal way less interactive.

So having builddir in ~/.cabal/config would be tricky.

TL;DR it's not as simple as adding a field to config. The builddir is very special, and should be available fast (in milliseconds).

@jneira jneira changed the title Supporting builddir in cabal.project Supporting builddir in cabal.project and global config Nov 14, 2021
@jneira
Copy link
Member

jneira commented Nov 14, 2021

Renaming the issue to include global config, as #6849, mentioning it, was closed as duplicate of this

@Mikolaj
Copy link
Member

Mikolaj commented Sep 4, 2023

See also #7924 for a much more modest related task.

@jeltsch
Copy link

jeltsch commented Feb 13, 2024

Why do some users even want to change the name of the build directory? At least I want to change it because the current name is bad in several ways:

  • The dist part is inappropriate, since the contents of this directory are not for distribution. Typically, the only thing that gets distributed is what’s generated with cabal sdist.
  • The newstyle part is inappropriate, because Nix-style local builds are by no means new. They are in fact quite old now.
  • The newstyle part is written as if “new style” was a single word instead of two.

Maybe just switch to a better name, like build-products, and the problem would be solved for the most part. I actually might prefer .build-products in order to hide the directory, but this wouldn’t work on Windows.

@jeltsch
Copy link

jeltsch commented Feb 13, 2024

By the way, from experiments I get the impression that the --builddir option can be passed before the subcommand and can be passed there even when it doesn’t make sense for the subcommand used (it’s possible to specify it also with update). Therefore, a workaround for us people being tired of seeing dist-newstyle in their directory listings 😉 could be to define a shell alias that expands to cabal --builddir=⟨desired-directory⟩. I’ll try this out.

@phadej
Copy link
Collaborator

phadej commented Feb 13, 2024

@jeltsch that is common misconception that dist-newstyle doesn't make sense for update. It does.

Even update command interacts with cabal.project: you can specify extra package repositories in cabal.project and these will be updated by cabal update. (and the parsing/resolving of cabal.project is cached in the --builddir).

@fgaz
Copy link
Member

fgaz commented Feb 14, 2024

@jeltsch see #5731 for switching to a better default

@jeltsch
Copy link

jeltsch commented Feb 16, 2024

@jeltsch that is common misconception that dist-newstyle doesn't make sense for update. It does.

Even update command interacts with cabal.project: you can specify extra package repositories in cabal.project and these will be updated by cabal update. (and the parsing/resolving of cabal.project is cached in the --builddir).

Interesting! What are actually the cabal-install subcommands that do not accept --builddir? So far, I’m only aware of init (for which in fact an alias as I’ve suggested above does not work).

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

No branches or pull requests