Commit Graph

7357 Commits

Author SHA1 Message Date
eugene yokota 85ab138187
Merge pull request #4901 from eatkins/no-exception-reload
Don't use exception for reloading
2019-07-29 16:08:16 -04:00
Ethan Atkins f5c8b8aad5 Don't use exception for reloading
I completely forgot about the StateTransform class which allows a task
to modify the state through its return value.
2019-07-26 15:03:32 -07:00
eugene yokota e3e2f29bdd
Merge pull request #4898 from eatkins/underscore
Use '_' instead of '$' in path names
2019-07-26 15:12:01 -04:00
Ethan Atkins f7f6c3edfe Use '_' instead of '$' in path names
The use of '$' in the path names for streams is a pain because, since
'$' is a special character in the shell, it makes it impossible to
directly copy and paste the paths. If we make this change, some builds
will be left with vestigial directories with $global and $build in them
until they run clean. It also would break any scripts that manually
delete these paths. That doesn't seem like a common use case, but it's
worth mentioning.
2019-07-25 14:07:44 -07:00
eugene yokota 646e408adb
Merge pull request #4894 from eed3si9n/wip/zipping
Fix java.util.zip.ZipException: duplicate entry
2019-07-23 10:19:40 -04:00
Eugene Yokota 9512a60fa6 Fix java.util.zip.ZipException: duplicate entry
Fixes #4889

#4329 switched from using Map to Seq during packaging. That allowed multiple files to be included with different paths, but it also started admitting duplicate files causing ZipException.
2019-07-23 01:28:32 -04:00
eugene yokota 8793034039
Merge pull request #4884 from naferx/msg
Minor clarification of logging message
2019-07-19 18:56:20 -04:00
Nafer Sanabria 3f3d7d47e3 Minor clarification of logging message 2019-07-19 06:01:15 -05:00
eugene yokota c76649024e
Merge pull request #4885 from eed3si9n/wip/creds
Fixes credential strictness
2019-07-18 22:43:47 -04:00
Eugene Yokota ef05e07cc5 Fixes credential strictness
Fixes #4882

In #4855 I inadvertently introduced `credential` strictness. This makes relaxes it again by ignoring if the credential file doesn't exist.
2019-07-18 18:30:30 -04:00
eugene yokota e1c7704e6c
Merge pull request #4876 from eatkins/bump-scalafmt
Bump scalafmt
2019-07-18 16:05:01 -04:00
Ethan Atkins a3ac4c76a6 Bump scalafmt
Intellij had issues resolving 2.0.0-RCX so it will be nice to be using
the latest.
2019-07-18 12:40:21 -07:00
eugene yokota 1ca295c2dc
Merge pull request #4877 from xuwei-k/oom-error-message
fix OutOfMemoryError message
2019-07-18 10:23:21 -04:00
eugene yokota abb72ce10d
Merge pull request #4874 from eatkins/drop-multi-command-validation
Don't validate multi commands
2019-07-17 23:40:17 -04:00
kenji yoshida 534fbfffbb
fix OutOfMemoryError message
s/werecommend/we recommend/
2019-07-18 12:05:46 +09:00
Ethan Atkins 125c4ba532 Don't validate multi commands
We tried to prevent users from doing something like running a multi
command "foo; bar" where foo is valid but bar is invalid so that we
wouldn't run foo only to discover bar was an invalid key. It isn't
possible to know in general if any command other than the first command
in a multi command is valid because it might update the state and add
the initially invalid command.

The validation caused the intellij plugin to not work with 1.3.0-RC3.
2019-07-17 19:27:07 -07:00
eugene yokota dd321da284
Merge pull request #4873 from eed3si9n/wip/useit
use sbt 1.3.0-RC3
2019-07-17 22:00:14 -04:00
Eugene Yokota 8bd1615e1f use sbt 1.3.0-RC3 2019-07-17 14:53:39 -04:00
eugene yokota 5c85ee572f
Merge pull request #4872 from eatkins/legacy-filters
Add scripted test for excludeFilter
2019-07-17 09:13:32 -04:00
Ethan Atkins 36f37d2fe3 Set linux compile options for make-clone
This scripted tests failed on the linux travis ci build with an
error around relocation. It said to use -fPIC in the compilation so
that's what I'm doing. It also said I needed to set the -std=c99 flag.
2019-07-16 22:18:45 -07:00
Ethan Atkins 995a1c6b8b Add nio tests to .travis.yml
These were inadvertently excluded when I refactored this file.
2019-07-16 20:48:07 -07:00
Ethan Atkins 1e1c92c57a Bump io version
This fixes the excludeFilter issue: #4868.
2019-07-16 20:35:44 -07:00
Ethan Atkins 69ed60653c Add scripted test for excludeFilter
It was reported in #4868 that exclueFilters didn't work correctly if
there was an || in the filter. This was an upstream issue in io, but
this commit adds a scripted test in sbt.
2019-07-16 20:35:44 -07:00
eugene yokota bf6814580c
Merge pull request #4871 from eatkins/relax-multi-parser
Relax strict commands
2019-07-16 22:18:04 -04:00
Ethan Atkins a93d9e77ad Relax strict commands
The recent changes to make the multi parser strict broke any multi
command, or alias, where the multi command contained a command or task
that was not yet defined, but was possibly added by reload. This was
reported as #4869. I had had to work around this issue in ScriptedTests
by running `reload` and `setUpScripted` separately instead of as a multi
command. This workaround doesn't work for aliasing boot, which has been
a recommended approach by Mark Harrah since 2011.

To fix this, I relax the strict parser. We don't require that the parser
be valid to create a multi command string. In the multiApplied state
transformation, however, we validate all of the commands up to 'reload'.
Since there is no way to validate any commands to the right of 'reload,
we optimistically allow those commands to run.

So long as there is no 'reload' in the multi commands, all of the
commands will be validated.
2019-07-16 15:17:21 -07:00
eugene yokota d4df289f2d
Merge pull request #4867 from eatkins/watch-trigger
Watch trigger
2019-07-15 21:40:28 -04:00
Ethan Atkins 6c4e23f77c Only persist file stamps in turbo mode
The use of the persistent file stamp cache between watch runs didn't
seem to cause any issues, but there was some chance for inconsistency
between the file stamp cache and the file system so it makes sense to
put it behind the turbo flag.

After changing the default, the watch/on-change scripted test started
failing. It turns out that the reason is that the file stamp cache
managed by the watch process was not pre-filled by task evaluation. For
this reason, the first time a source file was modified, it was treated
as a creation regardless of whether or not it actually was.

To fix this, I add logic to pre-fill the watch file stamp cache if we
are _not_ persisting the file stamps between runs.

I ran a before and after with the scala build performance benchmark tool
and setting the watchPersistFileStamps key to true reduced the median
run time by about 200ms in the non-turbo case.
2019-07-15 17:59:14 -07:00
Ethan Atkins 5e374a8e7d Move onEvent callback definition
It makes the file more readable to me to have this definition below the
definition of the FileEventMonitor.
2019-07-15 14:21:14 -07:00
Ethan Atkins 272508596a Use one observer for all aggregated watch tasks
There was a bug where sometimes a source file change would not trigger a
new build if the change occurred during a build. Based on the logs, it
seemed to be because a number of redundant events were generated for the
same path and they triggered the anti-entropy constraint of the file
event monitor.

To fix this, I consolidated a number of observers of the global file
tree repository into a single observer. This way, I am able to ensure
that only one event is generated per file event.

I also reworked the onEvent callback to only stamp the file once. It was
previously stamping the modified source file for all of the aggregated
tasks. In the sbt project running `~compile` meant that we were stamping
a source file 22 times whenever the file changed.

This actually uncovered a zinc issue though as well. Zinc computes and
writes the hash of the source file to the analysis file after
compilation has completed. If a source file is modified during
compilation, then the new hash is written to the analysis file even when
the compilation may have been made against the previous version of the
file. Zinc will then refuse to re-compile that file until another change
is made.

I manually verified that in the sbt project if I ran `~compile` before
this change and modified a file during compilation, then no event was
triggered (there was a log message about the event being dropped due to
the anti-entropy constraint though). After this change, if I changed a
file during compilation, it seemed to always trigger, but due to the
zinc bug, it didn't always re-compile.
2019-07-15 14:21:14 -07:00
eugene yokota 9b7ed84953
Merge pull request #4866 from eed3si9n/wip/bump
bump io, util, lm, and zinc
2019-07-15 15:36:57 -04:00
Eugene Yokota 0d1ce8ddcf bump io, util, lm, and zinc
Fixes #4841
Fixes #4011
2019-07-15 14:52:35 -04:00
eugene yokota a6da4b5b90
Merge pull request #4862 from eatkins/fix-warnings
Fix warnings
2019-07-15 12:57:16 -04:00
eugene yokota 82299c89ed
Merge pull request #4864 from eed3si9n/wip/developing
Split some of the developing related docs to DEVELOPING.md
2019-07-15 12:49:50 -04:00
Eugene Yokota 55d1cbbfe2 Split some of the developing related docs to DEVELOPING.md 2019-07-15 12:39:41 -04:00
eugene yokota e2921e70d2
Merge pull request #4861 from eatkins/reload-multi-commands
Reload multi commands
2019-07-13 20:11:23 -04:00
Ethan Atkins 055d7cd626 Remove unneeded cast
This was causing an abstract type pattern T is unchecked since it is
eliminated by erasure. It was unneeded because store.get[T] return
Option[(T, Long)]. I'm surprised that the compiler complained about
this.
2019-07-13 15:35:27 -07:00
Ethan Atkins f2c8d4f436 Fix implicit numeric widening warning
I noticed in CI that a warning was printed for this file about implicit
numeric widening in the GigaBytes case.
2019-07-13 15:20:39 -07:00
Ethan Atkins a071ce8224 Handle multi-command with reload correctly
@olegych reported that sbt would silently swallow the 'compile' command
in the multi-command, 'run;compile;reload'. I tracked this down to the
build source check. When the build has
Global / onChangedBuildSource := ReloadOnSourceChanges, the check build
sources command return a new state with "reload" prefixed. To actually
perform the reload, I returned this modified state with the prefixed
reload command.

There were two problems with this:
1) In the auto-reload case, the current command was not run after the
   reload
2) If the multi-command contained reload, the auto-reload check would
   have a false positive which triggered the bug in (1)

To fix this, I clear out the remaining commands before I run the check
command. That way, we know that if the remaining commands has a reload,
then it is an auto-reload. We then prefix the state with both the reload
and the current command.

I updated the scripted test for auto-reload to handle multi commands
containing reload.
2019-07-13 11:18:56 -07:00
Ethan Atkins 1b0159c547 Remove case in flatMap
I didn't notice that this was triggering a warning. I think at some
point I was actually doing something in the pattern match.
2019-07-13 10:46:25 -07:00
Ethan Atkins 8d20bd4c94
Merge pull request #4859 from eatkins/watch-defaults
Rework watch options
2019-07-12 15:17:54 -07:00
Ethan Atkins 263f00f3b2 Rework watch options
In this commit, I both restore some sbt 1.2.8 behavior and enhance the
api for setting keyboard shortcuts in watch. I change the default start
message to just show the watch count, the tasks that are being monitored
and, on a new line, the instructions to terminate the watch or show more
options.

Here's what it looks like:
[info] 1. Monitoring source files for spark/compile...
[info]    Press <enter> to interrupt or '?' for more options.
?
[info] Options:
[info]   <enter>  : interrupt (exits sbt in batch mode)
[info]   <ctrl-d> : interrupt (exits sbt in batch mode)
[info]   'r'      : re-run the command
[info]   's'      : return to shell
[info]   'q'      : quit sbt
[info]   '?'      : print options

I also made it so that the new options can be added (and old options
removed) with the watchInputOptions key. For example, to add an option
to reload the build with the key 'l', you could add
ThisBuild / watchInputOptions += Watch.InputOption('l', "reload", Watch.Reload)
to your global build.sbt.

After adding that to my global ~/sbt/1.0/global.sbt file, the output of
'?' became:
[info] Options:
[info]   <ctrl-d> : interrupt (exits sbt in batch mode)
[info]   <enter>  : interrupt (exits sbt in batch mode)
[info]   '?'      : print options
[info]   'l'      : reload
[info]   'q'      : quit sbt
[info]   'r'      : re-run the command
[info]   's'      : return to shell
2019-07-12 14:10:51 -07:00
eugene yokota 680659210f
Merge pull request #4848 from eatkins/background-copy-hash
Use last modified instead of hash
2019-07-12 15:24:09 -04:00
eugene yokota 3301bce3b8
Merge pull request #4850 from eatkins/in-memory-cache-store
Add support for in memory cache store
2019-07-12 15:23:40 -04:00
eugene yokota 2af6ad5713
Merge pull request #4858 from eed3si9n/wip/thisbuild
scope the reference of useSuperShell to ThisBuild
2019-07-12 15:21:57 -04:00
eugene yokota 5c3eff52d4
Merge pull request #4855 from eed3si9n/wip/credentials
add allCredentials to emulate credential registration
2019-07-12 15:21:36 -04:00
Eugene Yokota 00f7d1fab5 scope the reference of useSuperShell to ThisBuild
Fixes #4800
2019-07-12 11:34:39 -04:00
Eugene Yokota 9755234a16 address review 2019-07-12 10:52:02 -04:00
Ethan Atkins 0172d118af Add parser for file size
At the suggestion of @eed3si9n, instead of specifying the file cache
size in bytes, we now specify it in a formatted string. For example,
instead of specifying 128 megabytes in bytes (134217728), we can specify
it with the string "128M".
2019-07-11 17:45:16 -07:00
Ethan Atkins cad89d17a9 Add support for in memory cache store
It can be quite slow to read and parse a large json file. Often, we are
reading and writing the same file over and over even though it isn't
actually changing. This is particularly noticeable with the
UpdateReport*. To speed this up, I introduce a global cache that can be
used to read values from a CacheStore. When using the cache, I've seen
the time for the update task drop from about 200ms to about 1ms. This
ends up being a 400ms time savings for test because update is called for
both Compile / compile and Test / compile.

The way that this works is that I add a new abstraction
CacheStoreFactoryFactory, which is the most enterprise java thing I've
ever written. We store a CacheStoreFactoryFactory in the sbt State.
When we make Streams for the task, we make the Stream's
cacheStoreFactory field using the CacheStoreFactoryFactory. The
generated CacheStoreFactory may or may not refer to a global cache.

The CacheStoreFactoryFactory may produce CacheStoreFactory instances
that delegate to a Caffeine cache with a max size parameter that is
specified in bytes by the fileCacheSize setting (which can also be set
with -Dsbt.file.cache.size). The size of the cache entry is estimated by
the size of the contents on disk. Since we are generally storing things
in the cache that are serialized as json, I figure that this should be a
reasonable estimate. I set the default max cache size to 128MB, which is
plenty of space for the previous cache entries for most projects. If the
size is set to 0, the CacheStoreFactoryFactory generates a regular
DirectoryStoreFactory.

To ensure that the cache accurately reflects the disk state of the
previous cache (or other cache's using a CacheStore), the Caffeine cache
stores the last modified time of the file whose contents it should
represent. If there is a discrepancy in the last modified times (which
would happen if, say, clean has been run), then the value is read from
disk even if the value hasn't changed.

* With the following build.sbt file, it takes roughly 200ms to read and
parse the update report on my compute:

libraryDependencies += "org.apache.spark" %% "spark-sql" % "2.4.3"
libraryDependencies += "org.scalatest" %% "scalatest" % "3.0.1"

This is because spark-sql has an enormous number of dependencies and the
update report ends up being 3MB.
2019-07-11 17:45:16 -07:00
Eugene Yokota c31e0b6b55 add allCredentials to emulate credential registration
Fixes #4802

For Ivy integration sbt uses credential task in a peculiar way. 9fa25de84c/main/src/main/scala/sbt/Defaults.scala (L2271-L2275)

This lets the build user put `credential` task in various places, like metabuild or root project, but they would all act as if they were scoped globally. This PR adds `allCredentials` task to emulate the behavior to pass credentials into lm-coursier.
2019-07-11 14:15:33 -04:00