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

Clean cabal project #2097

Merged
merged 13 commits into from
Aug 18, 2021
Merged

Clean cabal project #2097

merged 13 commits into from
Aug 18, 2021

Conversation

jneira
Copy link
Member

@jneira jneira commented Aug 15, 2021

  • Clean main cabal.project, removing the shake-bench submodule and all the allow-newer
  • Create a specific project for run the benchmarks with all the workaround needed for build it:
    • All the allow-newer
    • And the submodule shake-bench, which now is being built, with all dependencies in all builds (with or without benchs)

The idea is backport thia to 1.3.0-hackage to help the hackage release (#1990)

@jneira jneira requested a review from pepeiborra August 15, 2021 15:01
Copy link
Collaborator

@pepeiborra pepeiborra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is shake-bench blocking the Hackage release? Please explain the issue and let's try to find a solution that doesn't introduce a new cabal-foo.project file.

Comment on lines 46 to 59
allow-newer:
diagrams-contrib:base,
diagrams-contrib:lens,
diagrams-contrib:random,
diagrams-core:base,
diagrams-core:lens,
diagrams-lib:base,
diagrams-lib:lens,
diagrams-postscript:base,
diagrams-postscript:lens,
diagrams-svg:base,
diagrams-svg:lens,
dual-tree:base,
svg-builder:base
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these still needed though? The upstream issue was fixed recently: diagrams/diagrams-core#112

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will try it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without the allow-newer for diagrams-core i got

PS D:\hls> cabal bench --project-file .\cabal-benchmark.project
Resolving dependencies...
cabal-3.4.0.0.exe: Could not resolve dependencies:
[__0] trying: ghc-api-compat-8.6.1 (user goal)
[__1] trying: base-4.14.2.0/installed-4.14.2.0 (dependency of ghc-api-compat)
[__2] trying: shake-bench-0.1.0.0 (user goal)
[__3] trying: diagrams-svg-1.4.3 (dependency of shake-bench)
[__4] trying: diagrams-core-1.5.0 (dependency of diagrams-svg)
[__5] next goal: Chart-diagrams (dependency of shake-bench)
[__5] rejecting: Chart-diagrams-1.9.3 (conflict: diagrams-core==1.5.0,
Chart-diagrams => diagrams-core>=1.3 && <1.5)
[__5] skipping: Chart-diagrams-1.9.2, Chart-diagrams-1.9.1,
Chart-diagrams-1.9, Chart-diagrams-1.8.3, Chart-diagrams-1.8.2,
Chart-diagrams-1.8.1, Chart-diagrams-1.8, Chart-diagrams-1.7.1,
Chart-diagrams-1.7, Chart-diagrams-1.6, Chart-diagrams-1.5.4,
Chart-diagrams-1.5.1, Chart-diagrams-1.5, Chart-diagrams-1.4,
Chart-diagrams-1.3.3, Chart-diagrams-1.3.2, Chart-diagrams-1.3.1,
Chart-diagrams-1.3, Chart-diagrams-1.2.4, Chart-diagrams-1.2.3,
Chart-diagrams-1.2.2, Chart-diagrams-1.2, Chart-diagrams-1.1,
Chart-diagrams-1.0 (has the same characteristics that caused the previous
version to fail: excludes 'diagrams-core' version 1.5.0)
[__5] fail (backjumping, conflict set: Chart-diagrams, diagrams-core,
shake-bench)
After searching the rest of the dependency tree exhaustively, these were the
goals I've had most trouble fulfilling: diagrams-core, base, Chart-diagrams,
shake-bench, diagrams-svg, ghc-api-compat
Try running with --minimize-conflict-set to improve the error message.

After trying several combinations i only get it to build enabling all of them maybe a more detailed analysis could make let us know the way to remove them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got to remove some other entries from allow-newer

@jneira
Copy link
Member Author

jneira commented Aug 16, 2021

How is shake-bench blocking the Hackage release? Please explain the issue and let's try to find a solution that doesn't introduce a new cabal-foo.project file.

It is not blocking the release directly but disturbing it with bench only releated allow-newer and a submodule, which is being taken in account for all builds even if they are not running or building benchmarks.
Otoh that makes you cant run the benchmarks against the hackage released version of ghcide.
I can restore all the workarounds needed to run the benchmarks in the main cabal.project file (with comments of course, to no be confused with other reason to add allow-newer), but i thought separate it would be a good motivation to try to remove the workarounds, which are since long time ago and seems to stay with us forever.

@jneira
Copy link
Member Author

jneira commented Aug 16, 2021

@pepeiborra ok, i've unified both cabal*.project again. Only would like to have some issue or plan to eventually remove the allow-newer i've not removed in this pr.

@jneira
Copy link
Member Author

jneira commented Aug 16, 2021

@Mergifyio backport 1.3.0-hackage

@mergify
Copy link
Contributor

mergify bot commented Aug 16, 2021

Command backport 1.3.0-hackage: pending

Waiting for the pull request to get merged

@pepeiborra
Copy link
Collaborator

The allow-newer for all the diagrams ecosystem should not be needed anymore, did you check?

@pepeiborra
Copy link
Collaborator

How is shake-bench blocking the Hackage release? Please explain the issue and let's try to find a solution that doesn't introduce a new cabal-foo.project file.

It is not blocking the release directly but disturbing it with bench only releated allow-newer and a submodule, which is being taken in account for all builds even if they are not running or building benchmarks.

The allow-newer might be superfluous, let's find out.

To avoid building shake-bench, just be more precise and list the targets that you wish to build: "cabal build haskell-language-server"

Otoh that makes you cant run the benchmarks against the hackage released version of ghcide.

Not sure what you mean. Care to elaborate,

I can restore all the workarounds needed to run the benchmarks in the main cabal.project file (with comments of course, to no be confused with other reason to add allow-newer), but i thought separate it would be a good motivation to try to remove the workarounds, which are since long time ago and seems to stay with us forever.

What workarounds are you referring to?

@jneira
Copy link
Member Author

jneira commented Aug 16, 2021

The allow-newer for all the diagrams ecosystem should not be needed anymore, did you check?

yeah, see #2097 (comment)

@jneira
Copy link
Member Author

jneira commented Aug 16, 2021

To avoid building shake-bench, just be more precise and list the targets that you wish to build: "cabal build haskell-language-server\

in my tests, no matter if you set a specific target or use --disable-benchmarks shake-bench is taken in account to compute the build plan, so it makes all builds fail if cabal does not find a plan including it

@jneira
Copy link
Member Author

jneira commented Aug 16, 2021

Not sure what you mean. Care to elaborate,

due to allow-never you can't run the benchmarks downloading the package(s) from hackage

@jneira
Copy link
Member Author

jneira commented Aug 16, 2021

What workarounds are you referring to?

for me allow-newer and remote source repo packages in cabal.project are workarounds to be removed asap

@pepeiborra
Copy link
Collaborator

To avoid building shake-bench, just be more precise and list the targets that you wish to build: "cabal build haskell-language-server\

in my tests, no matter if you set a specific target or use --disable-benchmarks shake-bench is taken in account to compute the build plan, so it makes all builds fail if cabal does not find a plan including it

Understood. I have opened #2101 to remove as many as possible, there are still 2 left.

@jneira jneira force-pushed the clean-cabal-project branch from 7e7fbf3 to 19bb487 Compare August 17, 2021 14:39
@jneira jneira requested a review from pepeiborra August 17, 2021 19:51
@jneira jneira added the merge me Label to trigger pull request merge label Aug 17, 2021
@mergify mergify bot merged commit 57ffd74 into haskell:master Aug 18, 2021
mergify bot pushed a commit that referenced this pull request Aug 18, 2021
* Remove superflous things

* Set index-state

* Add ghc-api-compat and remove shake-bench

* Correct comment about ghc-api-compat

* Add specific project file for benchs

* Use specific project file for benchmarks

* Ensure we use lsp-1.3.0.1

* Use lsp-types-1.3.0.1 for stack

* Use lsp-types-1.3.0.1 for default stack

* Remove some more allow-newer

* Use one cabal.project

Co-authored-by: Pepe Iborra <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 57ffd74)

# Conflicts:
#	cabal.project
@mergify
Copy link
Contributor

mergify bot commented Aug 18, 2021

Command backport 1.3.0-hackage: success

Backports have been created

mergify bot added a commit that referenced this pull request Aug 18, 2021
* Clean cabal project (#2097)

* Remove superflous things

* Set index-state

* Add ghc-api-compat and remove shake-bench

* Correct comment about ghc-api-compat

* Add specific project file for benchs

* Use specific project file for benchmarks

* Ensure we use lsp-1.3.0.1

* Use lsp-types-1.3.0.1 for stack

* Use lsp-types-1.3.0.1 for default stack

* Remove some more allow-newer

* Use one cabal.project

Co-authored-by: Pepe Iborra <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 57ffd74)

# Conflicts:
#	cabal.project

* Resolve conflicts

Co-authored-by: Javier Neira <[email protected]>
jneira added a commit that referenced this pull request Aug 18, 2021
* Remove superflous things

* Set index-state

* Add ghc-api-compat and remove shake-bench

* Correct comment about ghc-api-compat

* Add specific project file for benchs

* Use specific project file for benchmarks

* Ensure we use lsp-1.3.0.1

* Use lsp-types-1.3.0.1 for stack

* Use lsp-types-1.3.0.1 for default stack

* Remove some more allow-newer

* Use one cabal.project

Co-authored-by: Pepe Iborra <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 57ffd74)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants