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

cabal v2-sdist ignores --ignore-project #7965

Closed
andreabedini opened this issue Feb 10, 2022 · 17 comments
Closed

cabal v2-sdist ignores --ignore-project #7965

andreabedini opened this issue Feb 10, 2022 · 17 comments

Comments

@andreabedini
Copy link
Collaborator

Describe the bug

cabal sdist ignores --ignore-project

[~/tmp-pkg] cat cabal.project 
packages: tmp-pkg.cabal

source-repository-package
  type: git
  location: https://github.com/Quid2/flat
  tag: ee59880f47ab835dbd73bea0847dab7869fc20d8

[~/tmp-pkg] cabal v2-sdist --ignore-project
Cloning into '/home/andrea/tmp-pkg/dist-newstyle/src/flat-5d3bff3e9951fc1'...
remote: Enumerating objects: 3721, done.
remote: Counting objects: 100% (547/547), done.
remote: Compressing objects: 100% (362/362), done.
remote: Total 3721 (delta 358), reused 338 (delta 179), pack-reused 3174
Receiving objects: 100% (3721/3721), 1.23 MiB | 2.60 MiB/s, done.
Resolving deltas: 100% (2410/2410), done.
HEAD is now at ee59880 Don't use the unsafe decodeUtf8
Wrote tarball sdist to
/home/andrea/tmp-pkg/dist-newstyle/sdist/tmp-pkg-0.1.0.0.tar.gz

Clearly cabal has read cabal.project and decided to fetch the mentioned source-repository-package.

Expected behavior

I would expect it to not read cabal.project files at all.

System information

  • Ubuntu 21.10
  • cabal-install version 3.6.2.0
  • compiled using version 3.6.2.0 of the Cabal library
  • The Glorious Glasgow Haskell Compilation System, version 8.10.7
@Mikolaj
Copy link
Member

Mikolaj commented Feb 10, 2022

From what you say, it can be useful if cabal sdist takes --ignore-project into account? Because if not, it's cheaper to just remove the mention of the option (and broken code handling it, if any).

@phadej
Copy link
Collaborator

phadej commented Feb 10, 2022

It is occasionally useful. It seems to be broken though. I can reproduce.

https://github.com/haskell/cabal/blob/master/cabal-install/src/Distribution/Client/CmdSdist.hs#L141-L214

The withoutProject does something it shouldn't do.

@Mikolaj
Copy link
Member

Mikolaj commented Feb 10, 2022

Thanks. PR welcome.

@andreabedini
Copy link
Collaborator Author

To give a bit of context, I am writing a script to make sdists from a bunch of github repos. Given the repos might have their own cabal.project, cabal ends up pulling all the source-repository-packages. I find this undesiderable.

Does cabal actually need the source-repository-packages to make an sdist? I guess in case if a custom setup relies on those specific versions?

@gbaz
Copy link
Collaborator

gbaz commented Feb 10, 2022

You can always override those project files with a flag that sets the project file to a default one.

I think that we can't actually make --ignore-project work as designed for this command. It gets ignored on initial read of the project. But then to get the packages to sdist, we have to call establishProjectBaseContextWithRoot which in turn must call rebuildProjectConfig which needs a project file!

So 90% of commands can't take this flag as is, and have it stripped. I think there are very few use cases for this flag, and they can all be resolved by passing in another project file explicitly. If that workaround suffices, I vote we deprecate and then remove it.

(There is of course a way to make it work, but it would require substantial rearchitecting, and I question its utility).

@gbaz
Copy link
Collaborator

gbaz commented Feb 10, 2022

Here is the commands that cannot use the ignore project option already: https://github.com/haskell/cabal/search?q=removeIgnoreProjectOption

It includes "configure, build, exec, outdated". That's a pretty big list already, and I bet it works poorly and is possibly broken elsewhere that supports it, which is only "update, install, list-bin, run, repl" (and sdist, which we already know is broken).

@andreabedini
Copy link
Collaborator Author

Thanks for the analysis and the suggested workaround @gbaz

@gbaz
Copy link
Collaborator

gbaz commented Feb 11, 2022

Hrm. there is also a feature request for using ignore-project more widely: #7057

The argument there (which is good) is that this can bypass the behavior of searching ever upwards for a project file. So that makes a case that we should actually push through the more irritating change uniformly and make --ignore-project actually work for everything. 🤔

@andreabedini
Copy link
Collaborator Author

@gbaz I don't think passing --project-file works either. I tried cabal sdist --project-file=/dev/null and touch cabal.project.null && cabal sdist --project-file=cabal.project.null and the top level project still gets read.

A quick strace confirms.

[~/tmp] tree
.
├── cabal.project
├── pkg-a
│   ├── app
│   │   └── Main.hs
│   ├── CHANGELOG.md
│   └── pkg-a.cabal
└── pkg-b
    ├── app
    │   └── Main.hs
    ├── CHANGELOG.md
    └── pkg-b.cabal

4 directories, 7 files

[~/tmp] cd pkg-a/

[~/tmp/pkg-a] strace -f -o strace.out cabal sdist --project-file=/dev/null
Wrote tarball sdist to
/home/andrea/tmp/dist-newstyle/sdist/pkg-a-0.1.0.0.tar.gz

[~/tmp/pkg-a] grep cabal.project strace.out 
2885442 stat("/home/andrea/tmp/pkg-a/cabal.project", 0x7fff9df8d200) = -1 ENOENT (No such file or directory)
2885442 stat("/home/andrea/tmp/cabal.project", {st_mode=S_IFREG|0644, st_size=20, ...}) = 0
...

@gbaz
Copy link
Collaborator

gbaz commented Apr 12, 2022

In that case the issue I guess is that not all commands can take a --project-file flag properly, and I think we definitely should fix that.

@cbclemmer
Copy link
Collaborator

I think this may come down to the readProjectConfig function in ProjectConfig.hs. It looks like no matter if the --ignore-project flag is set or not, the rebuildProjectConfig function in run ProjectPlanning.hs is run with only the DistDirLayout object different based on the projectRoot parameter to the establishProjectBaseContextWithRoot function (this is based on the current dirctory if the flag is true and the current or parent directories if the flag is false). It looks like the readProjectConfig function always reads the local cabal.project file no matter how the flag is set and the project settings are merged with the defaults to have the effect of using the settings in the local cabal.project no matter what. What I can't figure out at the moment is whether readProjectConfig reads the cabal.project file and the foo.cabal file or just the cabal.project file.
I say this because I added a branching path based on whether the flag was set in the readProjectConfig function and the program acted like it couldn't find the foo.cabal file for some reason. I'll dig around more tomorrow but that's what I found today.
New code:
image
Before code change:
image
After code change:
image

@andreabedini
Copy link
Collaborator Author

andreabedini commented Apr 13, 2022

@Colton-Clemmer good investigation, I guess that's because the cabal packages to build are defined in the local cabal project file (in packages:). That must be why the function is actually called readProjectLocalConfigOrDefault. I think we need to replace local with a default one.

@cbclemmer
Copy link
Collaborator

I put up a pr for this issue that should hopefully resolve this problem #8109

cbclemmer added a commit to cbclemmer/cabal that referenced this issue May 2, 2022
mergify bot added a commit to cbclemmer/cabal that referenced this issue May 12, 2022
mergify bot added a commit that referenced this issue May 12, 2022
v2-sdist now respects --ignore-project
@andreasabel andreasabel added the re: --ignore-project Concerning flag `--ignore-project` label Aug 20, 2022
@andreasabel
Copy link
Member

With a PR merged, can this issue be closed?

@jneira jneira closed this as completed Aug 21, 2022
@phadej
Copy link
Collaborator

phadej commented Aug 21, 2022

Except now v2-sdist is broken as this change made it always require ghc, even with --ignore-project.

@andreasabel
Copy link
Member

Except now v2-sdist is broken as this change made it always require ghc, even with --ignore-project.

Is this taken care of by the following PR?

@jneira
Copy link
Member

jneira commented Aug 21, 2022

... and #8325 as issue. I suppose is good to link them here, thanks!

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

7 participants