Make etlas.dhall the default configuration file for etlas
jneira opened this issue ยท 40 comments
As described in typelead/eta#704 (comment).
Here we can track the progress and specific issues about. The plan would be:
- Make default build (
cabal new-build
) work withetlas.dhall
as the default configuration file- etlas will pick up
etlas.dhall
if present and ${project-name}.cabal otherwise
- etlas will pick up
- Make old-build (
cabal build
) works in a similar way - Analyze and improve performance.See
- Integrate the handling of etlas.dhall file in other etlas commands if needed: init (important!), info,
check, format (undocumented!), etc - Document the use of dhall to configure etlas in https://eta-lang.org/docs/user-guides/etlas-user-guide
I manged to implement a initial version that uses etlas.dhall
but testing it with https://github.com/jneira/wai-servlet/blob/master/etlas.dhall and i've got an error ๐ :
Error:
Dependency on unbuildable (i.e. 'buildable: False') library from base
In the stanza 'library'
In the inplace package 'wai-servlet-0.1.5.1'
@jneira Can you share the output with -v3
? It may be failing inside of etlas-cabal
.
Also can you add a debug log in etlas
that dumps the parsed GenericPackageDescription
from the etlas.dhall
file?
@jneira Any particular reason for:
jneira@5048002#diff-2d388570b233aa3bd5298e03db1e93c8L512
Shouldn't it be ["./etlas.dhall", "./*.cabal"]
?
That was my first version but then etlas requires both files and throwed an error if etlas.dhall was not present. Maybe it should change to make it required only one of them.
Oh right, that field is for required files. Then it looks ok.
- I've found the error is thrown here: https://github.com/typelead/etlas/blob/master/etlas-cabal/Distribution/Backpack/ConfiguredComponent.hs#L172
- At this point i've traced the package description of a succesful build with
.cabal
file and the erroneous one withetlas.dhall
(https://gist.github.com/jneira/58e57a6f186e55297e3197045f4788fb) - The diff that cause the error is in the
BuildDepends
field at package level: with.cabal
it is informed with the dependencies of all the components of the package, in the dhall case it is empty - In etlas/cabal that field has a doc annotation:
-- | YOU PROBABLY DON'T WANT TO USE THIS FIELD. This field is
-- special! Depending on how far along processing the
-- PackageDescription we are, the contents of this field are
-- either nonsense, or the collected dependencies of *all* the
-- components in this package. buildDepends is initialized by
-- 'finalizePD' and 'flattenPackageDescription';
-- prior to that, dependency info is stored in the 'CondTree'
-- built around a 'GenericPackageDescription'. When this
-- resolution is done, dependency info is written to the inner
-- 'BuildInfo' and this field. This is all horrible, and #2066
-- tracks progress to get rid of this field.
- So
DhallToCabal
ignores that field and left it empty - I guess etlas is filling it at some point for
.cabal
format but no withetlas.dhall
- Another diff that maybe is related: the field
custom-setup
has valueNothing
in the.cabal
version butJust (SetupBuildInfo {setupDepends = [], defaultSetupDepends = True})
, although the default value for it indhall-to-etlas
is[] : Optional types.CustomSetup
(and in the sourceDhall.field "custom-setup" ( Dhall.maybe setupBuildInfo )
)
@jneira Maybe you can try populating buildDepends
in dhall-to-etlas
temporarily to see if that fixes it?
Mmmm, that would be a solution but as i have to see how etlas does it maybe i can see where it does and hopefully fix in etlas itself, if possible
Sure that works too!
The problem seems to be in the initial parse of config files, comparing the GenericPackageDescription
generated by cabal internal parser and dhall-to-etlas we can check that dhall-to-etlas is not filling the CondNode.condTreeConstraints
in any component. There is the info of conditional dependencies of the component.
That difference was not catched by dhall-to-etlas tests cause they are testing the output of showGenericDescription
and it is equal without condTreeConstraints
info
Does filling in those fields properly fix the issue you were hitting?
@rahulmutt yeah, i think dhall-to-etlas should fill it when parsing the file in this function.
In the meanwhile i've change the code to reparse the GenericPackageDescription from dhall with Cabal itself although it is terribly inefficient.
Otoh thinking twice about defaultImplicitProjectConfig.defaultPackages
i think we cant left it empty and use cause using projectPackagesOptional
makes cabal build the package twice, one using dhall fille and another one using the cabal file. As it uses "glob" to find files we cant use (./etlas.dhall)|(./*.cabal)
to use one file or another.
@jneira That workaround seems OK for now, let's just focus on correctness first and then we'll worry about performance.
To fix that problem of building package twice, we need to do a filtering before returning with the concat
arguments in findProjectPackages
. The filtering works like this:
Find all ProjectPackageLocalCabalFile
and ProjectPackageLocalDhallFile
that share the same parent directory. If such pairs are found, filter out the ProjectPackageLocalCabalFile
and go with the ProjectPackageLocalDhallFile
.
Hi, i have a initial working version of etlas with dhall integration
I've tested most of etlas commands with this repo: https://github.com/jneira/eta-test/tree/test-dhall. The tests had been simple: run the command without args but -v3
with dhall file, cabal one and both, without dependencies and with dhall/aeson.
The checklist of dhall integration by command:
- New build
- get: not applicable
- init: not implemented
- configure: tested
- build: tested
- docs: same behaviour
etlas.exe: The program 'etadoc' version >=0.1 is required but it could not be found.
- deps: tested
- clean: tested
- run: tested
- repl: tested
- test: to test
- bench: to test
- check: not implemented
- sdist: tested
- bdist: to test
- upload: not implemented (maybe not needed?)
- report: not implemented (maybe not needed?)
- freeze: tested
- gen-bounds: same behaviour (
etlas.exe: Could not resolve dependencies
) - outdated: tested
- format: not implemented
- hscolour: not applicable
- copy: not tested (maybe not needed?)
- register: same behaviour
eta.exe: can't find a package database at dist\package.conf.inplace
- Old build
- reconfigure: tested
- old-build: tested
- old-configure: tested
- old-deps: tested
- old-docs: same behaviour
etlas.exe: The program 'etadoc' version >=0.1 is required but it could not be found.
- old-repl: tested
- old-run: tested
- old-install: tested
- old-test: to test
- old-freeze: tested (needs old-configure)
Pending:
- avoid reparse of package config with cabal making dhall-to-etlas build the complete
GenericPackageDescription
: dhall-lang/dhall-to-cabal#130 - cache the dhall file, using its own cache mechanism with hash. Performance of parsing it's very poor, although prelude and type external files are already cached
This is great news @jneira! :D
Performance of parsing dhall config files:
- Non-cached version ๐
D:\ws\eta\eta-test>etlas clean -v3
....
Reading package configuration from .\etlas.dhall
Configuration readed in 10.9980705 seconds
....
- Import with hash non-cached ๐
D:\ws\eta\eta-test>type etlas.dhall
./etlas.raw.dhall sha256:e036b394eb43c044f3ef6fbcbd92c4ddac88ad4ce2bf8c5ac2cbba66d0e23185
D:\ws\eta\eta-test>etlas clean -v3
....
Reading package configuration from .\etlas.dhall
Configuration readed in 17.3785114 seconds
....
- Import with hash cached ๐
D:\ws\eta\eta-test>etlas clean -v3
....
Reading package configuration from .\etlas.dhall
Configuration readed in 0.1092007 seconds
....
So we must use the cached version whenever possible. The computation of the hash takes the same time that parsing itself (cause dhall caches the normalized version) so we should find out the way to get the content and the hash at same time, in one step. Then we should save the import hashed dhall file in dist/cache/import.etlas.dhall
and use it while the modification time of the original etlas.dhall
file is older.
I am afraid that those 17 seconds are unavoidable in each etlas.dhall file change but let's hope the performance of dhall parsing improves.
Great analysis @jneira! That 17 seconds is a bit much. I think a performance target should be to get it down to at least 3 seconds - it's OK if it's a couple seconds slower than the .cabal
file format since it adds more power/re-use.
Users will typically not change their .cabal files very often, but they shouldn't have to think twice to make changes b/c of perf issues!
Well, another option is to use a derived .cabal
file as "cache" file cause it is parsed faster. In that way users will have to wait 10 seconds in each dhall file change.
After updating etlas to use dhall-1.18.0
and dhall-to-etlas-1.4.0.0
the performance has improved but continue being not satisfactory:
- Without caching
dhall-to-etlas
prelude noretlas.dhall
D:\ws\eta\eta-test>etlas clean -v3
no user package environment file found at D:\ws\eta\eta-test
Reading package configuration from .\etlas.dhall
Configuration readed in 8.1900525 seconds
Using internal setup method with build-type Simple and args:
["clean","--verbose=3","--builddir=dist"]
cleaning...
- With
dhall-to-etlas
prelude cached anddhall.etlas
not cached
D:\ws\eta\eta-test>etlas clean -v3
no user package environment file found at D:\ws\eta\eta-test
Reading package configuration from .\etlas.dhall
Configuration readed in 6.3024404 seconds
Using internal setup method with build-type Simple and args:
["clean","--verbose=3","--builddir=dist"]
cleaning...
- First run after setting hash for
etlas.dhall
(the usual initial case after changeetlas.dhall
)
D:\ws\eta\eta-test>etlas clean -v3
no user package environment file found at D:\ws\eta\eta-test
Reading package configuration from .\etlas.dhall
Configuration readed in 10.3272662 seconds
Using internal setup method with build-type Simple and args:
["clean","--verbose=3","--builddir=dist"]
cleaning...
- With
etlas.dhall
already cached
D:\ws\eta\eta-test>etlas clean -v3
no user package environment file found at D:\ws\eta\eta-test
Reading package configuration from .\etlas.dhall
Configuration readed in 0.1092007 seconds
Using internal setup method with build-type Simple and args:
["clean","--verbose=3","--builddir=dist"]
cleaning...
Although times are better, i think the threshold should be 5 seconds for the worst case for being acceptable, that is, when user changes the etlas.dhall
file and etlas has to force caching freezing ad reading it.
Hi, i've done some tests with the new config files, <pkg-name>.etlas
and etlas.dhall
:
- Uploading a package with each file as main config file in my fork of
etlas-index
: https://github.com/jneira/etlas-index/tree/master/packages/eta-test- Version
0.1.0.0
witheta-test.etlas
- Version
0.2.0.0
withetlas.dhall
- Version
- Trying to execute
etlas old-install
with each version - Trying to execte
etlas build
on a package with both versions ofeta-test
as dependency
The unique test that fails is etlas old-install eta-test-0.2.0.0
(the version with etlas.dhall
):
- etlas log
Copying C:\Users\user\AppData\Roaming\etlas\packages\jneira\eta-test\0.2.0.0 to
C:\TEMP\etlas-tmp-8040...
copy directory
'C:\Users\user\AppData\Roaming\etlas\packages\jneira\eta-test\0.2.0.0' to
'C:\TEMP\etlas-tmp-8040'.
creating C:\TEMP\etlas-tmp-8040
creating C:\TEMP\etlas-tmp-8040\src
creating C:\TEMP\etlas-tmp-8040\java
creating C:\TEMP\etlas-tmp-8040\app
creating C:\TEMP\etlas-tmp-8040\.git
creating C:\TEMP\etlas-tmp-8040\.git\objects\fd
creating C:\TEMP\etlas-tmp-8040\.git\objects
creating C:\TEMP\etlas-tmp-8040\.git\objects\fd
creating C:\TEMP\etlas-tmp-8040\.git\objects\e1
creating C:\TEMP\etlas-tmp-8040\.git\objects\d0
creating C:\TEMP\etlas-tmp-8040\.git\objects\be
creating C:\TEMP\etlas-tmp-8040\.git\objects\ba
creating C:\TEMP\etlas-tmp-8040\.git\objects\b9
creating C:\TEMP\etlas-tmp-8040\.git\objects\b4
creating C:\TEMP\etlas-tmp-8040\.git\objects\7a
creating C:\TEMP\etlas-tmp-8040\.git\objects\6d
creating C:\TEMP\etlas-tmp-8040\.git\objects\63
creating C:\TEMP\etlas-tmp-8040\.git\objects\55
creating C:\TEMP\etlas-tmp-8040\.git\objects\51
creating C:\TEMP\etlas-tmp-8040\.git\objects\27
creating C:\TEMP\etlas-tmp-8040\.git\objects\24
creating C:\TEMP\etlas-tmp-8040\.git\objects\03
creating C:\TEMP\etlas-tmp-8040\.git\logs
creating C:\TEMP\etlas-tmp-8040\.git\info
creating C:\TEMP\etlas-tmp-8040\.git\hooks
Configuring eta-test-0.2.0.0...
Reading package configuration from C:\TEMP\etlas-tmp-8040\etlas.dhall
Configuration readed in 6.24004e-2 seconds
Writing derived cabal file from dhall file: dist\etlas.dhall.cabal
Using self-exec internal setup method with build-type Simple and args:
["act-as-setup","--build-type=Simple","--","configure","--verbose=3","--builddir
=dist","--eta","--prefix=C:\\Users\\user\\AppData\\Roaming\\etlas","--bindir=C:\\
Users\\user\\AppData\\Roaming\\etlas\\bin","--libdir=C:\\Users\\user\\AppData\\Roa
ming\\etlas","--libsubdir=eta-0.8.6.3\\eta-test-0.2.0.0-Ifk6o0gD3O9BytxAzLdR9E",
"--dynlibdir=C:\\Users\\user\\AppData\\Roaming\\etlas\\eta-0.8.6.3","--libexecdir
=C:\\Users\\user\\AppData\\Roaming\\etlas\\eta-test-0.2.0.0-Ifk6o0gD3O9BytxAzLdR9
E","--datadir=C:\\Users\\user\\AppData\\Roaming\\etlas","--datasubdir=eta-0.8.6.3
\\eta-test-0.2.0.0","--docdir=C:\\Users\\user\\AppData\\Roaming\\etlas\\doc\\eta-
0.8.6.3\\eta-test-0.2.0.0","--htmldir=C:\\Users\\user\\AppData\\Roaming\\etlas\\d
oc\\eta-0.8.6.3\\eta-test-0.2.0.0\\html","--haddockdir=C:\\Users\\user\\AppData\\
Roaming\\etlas\\doc\\eta-0.8.6.3\\eta-test-0.2.0.0\\html","--etadocdir=C:\\Users
\\user\\AppData\\Roaming\\etlas\\doc\\eta-0.8.6.3\\eta-test-0.2.0.0\\html","--sys
confdir=C:\\Users\\user\\AppData\\Roaming\\etlas\\etc","--user","--ipid=eta-test-
0.2.0.0-Ifk6o0gD3O9BytxAzLdR9E","--extra-prog-path=C:\\Users\\user\\AppData\\Roam
ing\\etlas\\bin","--dependency=base=base-4.11.1.0-9g5RhDO8RwrLhMSXPi8Eih","--dep
endency=base=base-4.11.1.0-9g5RhDO8RwrLhMSXPi8Eih","--disable-tests","--exact-co
nfiguration","--disable-benchmarks","--cabal-file","dist\\etlas.dhall.cabal"]
D:\Trabajo\NO-SALVAR\jneira\bin\etlas.exe act-as-setup --build-type=Simple --
configure --verbose=3 --builddir=dist --eta
--prefix=C:\Users\user\AppData\Roaming\etlas
--bindir=C:\Users\user\AppData\Roaming\etlas\bin
--libdir=C:\Users\user\AppData\Roaming\etlas
--libsubdir=eta-0.8.6.3\eta-test-0.2.0.0-Ifk6o0gD3O9BytxAzLdR9E
--dynlibdir=C:\Users\user\AppData\Roaming\etlas\eta-0.8.6.3
--libexecdir=C:\Users\user\AppData\Roaming\etlas\eta-test-0.2.0.0-Ifk6o0gD3O9Bytx
AzLdR9E
--datadir=C:\Users\user\AppData\Roaming\etlas
--datasubdir=eta-0.8.6.3\eta-test-0.2.0.0
--docdir=C:\Users\user\AppData\Roaming\etlas\doc\eta-0.8.6.3\eta-test-0.2.0.0
--htmldir=C:\Users\user\AppData\Roaming\etlas\doc\eta-0.8.6.3\eta-test-0.2.0.0\ht
ml
--haddockdir=C:\Users\user\AppData\Roaming\etlas\doc\eta-0.8.6.3\eta-test-0.2.0.0
\html
--etadocdir=C:\Users\user\AppData\Roaming\etlas\doc\eta-0.8.6.3\eta-test-0.2.0.0\
html
--sysconfdir=C:\Users\user\AppData\Roaming\etlas\etc --user
--ipid=eta-test-0.2.0.0-Ifk6o0gD3O9BytxAzLdR9E
--extra-prog-path=C:\Users\user\AppData\Roaming\etlas\bin
--dependency=base=base-4.11.1.0-9g5RhDO8RwrLhMSXPi8Eih
--dependency=base=base-4.11.1.0-9g5RhDO8RwrLhMSXPi8Eih --disable-tests
--exact-configuration --disable-benchmarks --cabal-file dist\etlas.dhall.cabal
Redirecting build log to {handle:
C:\Users\user\AppData\Roaming\etlas\logs\eta-0.8.6.3\eta-test-0.2.0.0-Ifk6o0gD3O9
BytxAzLdR9E.log}
- eta log
?callStack, called at .\Distribution\Compat\Stack.hs:45:13 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.Compat.Stack
callStack, called at .\Distribution\Simple\Utils.hs:591:44 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.Simple.Utils
withCallStackPrefix, called at .\Distribution\Simple\Utils.hs:628:7 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.Simple.Utils
withMetadata, called at .\Distribution\Simple\Utils.hs:353:15 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.Simple.Utils
die', called at .\Distribution\PackageDescription\Parsec.hs:84:7 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.PackageDescription.Parsec
readAndParseFile, called at .\Distribution\PackageDescription\Parsec.hs:96:33 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.PackageDescription.Parsec
readGenericPackageDescription, called at .\Distribution\Simple.hs:250:19 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.Simple
confPkgDescr, called at .\Distribution\Simple.hs:219:33 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.Simple
configureAction, called at .\Distribution\Simple.hs:184:19 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.Simple
etlas.exe: Error Parsing: file "dist\etlas.dhall.cabal" doesn't exist. Cannot
continue.
etlas: Leaving directory 'C:\TEMP\etlas-tmp-8040'
So the dhall file is parsed and used, the derived cabal file writed (not error in that step) but eta
does not find it. Maybe the write is failing silently cause the dist folder soes not exist... to investigate and fix ๐ค
From a cursory glance at the logs, I notice that the dhall parsing code uses relative paths, so perhaps the derived cabal file is being written to the dist
directory of the current directory of the etlas
process? Perhaps you need to be writing to C:\TEMP\etlas-tmp-8040\dist\etlas.dhall.cabal
?
etlas build
works with a dist
folder relative to the current directory, so it makes sense that it works. But there is a case in which it doesn't:
directory structure
- eta-test
- eta-test2
- etlas.dhall (some basic stub file)
- cabal.project
cabal.project
packages: eta-test2
Run the following:
cd eta-test2
etlas build
In this case, it should create the dist
directory inside of eta-test
and not inside of eta-test2
since new-build
system uses a centralized dist
directory for all the packages you're building for a given cabal.project
file. Hence, if relative directory mismatch is the problem, probably this should fail as well? Unless the new-build codepath for dhall passes in the paths properly.
The performance of near version of dhall-to-etlas 1.4.0.0 starts to be reasonable:
golden tests
dhall-to-cabal
wai-servlet: OK (3.48s)
wai-servlet-handler-jetty: OK (3.34s)
nested-conditions: OK (3.07s)
mixins-no-signatures: OK (2.97s)
map-source-repo: OK (2.99s)
gh-55: OK (3.23s)
gh-53: OK (3.13s)
empty-package: OK (0.59s)
dhall-to-etlas: OK (4.22s)
default-license-2.2: OK (2.84s)
conditional-dependencies: OK (3.65s)
compiler-options-order: OK (3.48s)
allrightsreserved-2.2: OK (2.92s)
allrightsreserved-2.0: OK (2.84s)
๐
That's amazing @jneira! Thank for you all the work for making dhall practical :)
With #83 we'll got the new performance improvements and the fix to etlas old-install
of remote packages with etlas.dhall
files.
The performance of etlas using dhall has improved and it is below 5 seconds:
- Without caching
C:\Users\Javier\dev\lang\eta\eta-test>etlas clean -v3
...
Reading package configuration from .\etlas.dhall
Configuration readed in 4.75 seconds
- First time after using hash
C:\Users\Javier\dev\lang\eta\eta-test>etlas clean -v3
...
Reading package configuration from .\etlas.dhall
Configuration readed in 8.6875 seconds
- After caching
C:\Users\Javier\dev\lang\eta\eta-test>etlas clean -v3
...
Reading package configuration from .\etlas.dhall
Configuration readed in 0.296875 seconds
So it is way better but not enough to use it without cache.
However we need to cache it in each change and it would be still slow. Possible solutions:
- Use a derived cabal file as "cache" cause it is fast to write and read
- Launch the dhall caching in parallel in the first build after change, to not slow down.
However we need to cache it in each change and it would be still slow. Possible solutions:
* Use a derived cabal file as "cache" cause it is fast to write and read
This sounds like a great idea.
* Launch the dhall caching in parallel in the first build after change, to not slow down.
Can you elaborate on this? So are you saying that it's taking 4.75 seconds because caching is happening in addition to parsing? w/o caching how much time would it take?
The second option would be read the changed dhall file (in 4.75 s), start to build immediately in one thread and start another one to cache the readed data (that hopefully will end in 4 seconds aprox.)
Not sure how can interact both processes and maybe it is too much work to avoid 4 seconds.
Finally we've implemented a cache of etlas.dhall using builtin dhall cache mechanism: #88
But the cache invalidation depends on the hash of dhall.etlas
, each change in the file will trigger the read and cache steps.
@rahulmutt has made good points about the usage of the dhall format in etlas
config context: see typelead/eta#946
possible improvements
etlas freeze --format=dhall
: make aetlas.dhall.freeze
with all dependencies pinned to a single version and all dhall imports hashed (with an optional argument--in-place
to update the default one?)etlas format
: apply dhall format toetlas.dhall
etlas check
: apply dhall lint toetlas.dhall
to get possible warnings (without changing it)- using templates in
etlas init
to generate the defaultetlas.dhall
. The templates and possible values would be dhall files themselves. Ideallyetlas init
should use it on the fly to ask the questions in interactive mode. Maybe this one could be a separate library/executable
To make etlas.dhall
more resilient and make it work oflline we should (as suggested in dhall-lang/dhall-lang#378):
- Download and put in dhall cache all core imports (types, prelude and default dependencies) inside
~/.etlas
- Replace the file remote imports matching the urls for the local ones before evaluate
etlas.dhall
@jneira Sounds like a nice idea. Do you think this would make the performance even better?
Definitely it will for dependencies.dhall
cause it can change and we should not freeze with a hash by default. I was thinking in downloading it as part of etlas update
.
Perhaps the easiest way is to just add it to etlas-index
? No need to add additional logic other than to cache it.
I've requested a pr to bump up dhall version to 1.23: #102
This version has improved nomalization perf so:
Reading and caching package configuration from dhall file: .\etlas.dhall
Configuration readed in 4.836031 seconds
Mmm, i think only left use a local copy of imports as suggested in #66 (comment)