Skip to content

Commit

Permalink
Allow target to override existing package #2028
Browse files Browse the repository at this point in the history
  • Loading branch information
mgsloan committed Apr 12, 2016
1 parent d8a3037 commit ee8ad26
Showing 1 changed file with 8 additions and 20 deletions.
28 changes: 8 additions & 20 deletions src/Stack/Build/Target.hs
Expand Up @@ -149,28 +149,16 @@ resolveIdents _ _ _ (ri, RTPackageComponent x y) = Right ((ri, RTPackageComponen
resolveIdents _ _ _ (ri, RTComponent x) = Right ((ri, RTComponent x), Map.empty)
resolveIdents _ _ _ (ri, RTPackage x) = Right ((ri, RTPackage x), Map.empty)
resolveIdents snap extras locals (ri, RTPackageIdentifier (PackageIdentifier name version)) =
case mfound of
Just (foundPlace, foundVersion) | foundVersion /= version -> Left $ T.pack $ concat
[ "Specified target version "
, versionString version
, " for package "
, packageNameString name
, " does not match "
, foundPlace
, " version "
, versionString foundVersion
]
_ -> Right
( (ri, RTPackage name)
, case mfound of
-- Add to extra deps since we didn't have it already
Nothing -> Map.singleton name version
-- Already had it, don't add to extra deps
Just _ -> Map.empty
)
Right ((ri, RTPackage name), newExtras)
where
newExtras =
case mfound of
-- If the found version matches, no need for an extra-dep.
Just (_, foundVersion) | foundVersion == version -> Map.empty
-- Otherwise, if there is no specified version or a
-- mismatch, add an extra-dep.
_ -> Map.singleton name version
mfound = mlocal <|> mextra <|> msnap

mlocal = (("local", ) . lpvVersion) <$> Map.lookup name locals
mextra = ("extra-deps", ) <$> Map.lookup name extras
msnap = ("snapshot", ) <$> Map.lookup name snap
Expand Down

3 comments on commit ee8ad26

@borsboom
Copy link
Contributor

Choose a reason for hiding this comment

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

@mgsloan: looks like this change may cause integration tests to fail:

  1) 606-local-version-not-exist
       uncaught exception: TestFailure (Exited with ExitFailure 1

       stdout:


       stderr:
       Running: /var/lib/jenkins/jobs/stack-integration-tests/workspace/.stack-work/install/x86_64-linux/lts-5.3/7.10.3/bin/stack build files-3
       Downloading lts-4.0 build plan ...
       Downloaded lts-4.0 build plan.
       Caching build plan
       [1 of 1] Compiling Main             ( /tmp/stack24507/Setup.hs, /tmp/stack24507/Setup.o )
       Linking /tmp/stack-integration-home19922/.stack/setup-exe-cache/x86_64-linux/tmp-setup-Simple-Cabal-1.22.5.0-ghc-7.10.3 ...
       files-0.1.0.0: configure
       Configuring files-0.1.0.0...
       files-0.1.0.0: build
       Preprocessing library files-0.1.0.0...
       [1 of 1] Compiling Lib              ( src/Lib.hs, .stack-work/dist/x86_64-linux/Cabal-1.22.5.0/build/Lib.o )
       In-place registering files-0.1.0.0...
       files-0.1.0.0: copy/register
       Installing library in
       /tmp/stack-integration-606-local-version-not-exist19922/.stack-work/install/x86_64-linux/lts-4.0/7.10.3/lib/x86_64-linux-ghc-7.10.3/files-0.1.0.0-E3PQgkWJYFuB4nbBz80a74
       Registering files-0.1.0.0...
       Main.hs: stack was supposed to fail, but didn't
       )

@luigy
Copy link
Contributor

@luigy luigy commented on ee8ad26 Apr 13, 2016

Choose a reason for hiding this comment

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

looks like this just exposed the existing issue of extra-deps missing from index #1521 get silently ignored

It should indeed fail and we were discussing about it in #1858 :)

@mgsloan
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While this indeed could be covered by such extra-deps checks, I think it's best to special case this one during target parsing. That way the error message can be clear. I've made that change here 7262dd2

Please sign in to comment.