Rome icon indicating copy to clipboard operation
Rome copied to clipboard

[WIP] Optimistic download

Open tmspzz opened this issue 5 years ago • 12 comments

As per #181 , if a version file exists trust that everything else is there too.

tmspzz avatar May 15 '19 17:05 tmspzz

22 Warnings
:warning: PR is classed as Work in Progress
:warning: src/CommandParsers.hs#L112 - Found Use <$>
pure Download <*> udcPayloadParser

Why Not

Download <$> udcPayloadParser
:warning: src/Lib.hs#L256 - Found Reduce duplication
let finalRepositoryMapEntries
      = if _noIgnore noIgnoreFlag then repositoryMapEntries else
          repositoryMapEntries `filterRomeFileEntriesByPlatforms`
            ignoreMapEntries
let repositoryMap = toRepositoryMap finalRepositoryMapEntries
let reverseRepositoryMap
      = toInvertedRepositoryMap finalRepositoryMapEntries
let finalIgnoreNames
      = if _noIgnore noIgnoreFlag then [] else ignoreFrameworks

Why Not

Combine with src/Lib.hs:359:5
:warning: src/Lib.hs#L275 - Found Reduce duplication
let filteredCurrentMapEntries
      = currentMapEntries `filterRomeFileEntriesByPlatforms`
          ignoreMapEntries
let currentFrameworks
      = concatMap (snd . romeFileEntryToTuple) filteredCurrentMapEntries
let currentFrameworkVersions
      = map (flip FrameworkVersion currentVersion) currentFrameworks
let currentInvertedMap
      = toInvertedRepositoryMap filteredCurrentMapEntries

Why Not

Combine with src/Lib.hs:400:15
:warning: src/Lib.hs#L295 - Found Redundant bracket
if _noSkipCurrent noSkipCurrentFlag then
  (currentFrameworkVersions `filterOutFrameworksAndVersionsIfNotIn`
     finalIgnoreNames)
  else []

Why Not

if _noSkipCurrent noSkipCurrentFlag then
  currentFrameworkVersions `filterOutFrameworksAndVersionsIfNotIn`
    finalIgnoreNames
  else []
:warning: src/Lib.hs#L973 - Found Reduce duplication
dwarfUUIDs <- dwarfUUIDsFrom (frameworkDirectory </> fwn)
maybeUUIDsArchives <- liftIO $
                        forM dwarfUUIDs $
                          \ dwarfUUID ->
                            runMaybeT $
                              do dwarfArchive <- exceptToMaybeT $
                                                   createZipArchive (bcSymbolMapPath dwarfUUID)
                                                     verbose
                                 return (dwarfUUID, dwarfArchive)
unless skipLocalCache $
  forM_ maybeUUIDsArchives $
    mapM $
      \ (dwarfUUID, dwarfArchive) ->
        maybe (return ()) liftIO $
          runReaderT <$>
            (saveBcsymbolmapToLocalCache <$> mlCacheDir <*> Just dwarfUUID <*>
               Just dwarfArchive
               <*> Just reverseRomeMap
               <*> Just fVersion
               <*> Just platform)
            <*> Just (cachePrefix, s, verbose)

Why Not

Combine with src/Lib.hs:1189:7
:warning: src/Lib.hs#L1008 - Found Reduce duplication
frameworkNameWithFrameworkExtension = appendFrameworkExtensionTo f
platformBuildDirectory
  = carthageArtifactsBuildDirectoryForPlatform platform f
frameworkDirectory
  = platformBuildDirectory </> frameworkNameWithFrameworkExtension
dSYMNameWithDSYMExtension
  = frameworkNameWithFrameworkExtension <> ".dSYM"
dSYMdirectory
  = platformBuildDirectory </> dSYMNameWithDSYMExtension
bcSymbolMapPath d
  = platformBuildDirectory </> bcsymbolmapNameFrom d

Why Not

Combine with src/Lib.hs:1088:3
:warning: src/Lib.hs#L1008 - Found Reduce duplication
frameworkNameWithFrameworkExtension = appendFrameworkExtensionTo f
platformBuildDirectory
  = carthageArtifactsBuildDirectoryForPlatform platform f
frameworkDirectory
  = platformBuildDirectory </> frameworkNameWithFrameworkExtension
dSYMNameWithDSYMExtension
  = frameworkNameWithFrameworkExtension <> ".dSYM"
dSYMdirectory
  = platformBuildDirectory </> dSYMNameWithDSYMExtension
bcSymbolMapPath d
  = platformBuildDirectory </> bcsymbolmapNameFrom d

Why Not

Combine with src/Lib.hs:1224:3
:warning: src/Lib.hs#L1282 - Found Reduce duplication
saveBinaryToLocalCache lCacheDir versionFileBinary
  (prefix </> versionFileRemotePath)
  versionFileName
  verbose
liftIO $ saveBinaryToFile versionFileBinary versionFileLocalPath
sayFunc $
  "Copied " <> versionFileName <> " to: " <> versionFileLocalPath

Why Not

Combine with src/Lib.hs:1801:17
:warning: src/Lib.hs#L1297 - Found Reduce duplication
versionFileName
  = versionFileNameForProjectName $ fst projectNameAndVersion
versionFileLocalPath = carthageBuildDirectory </> versionFileName
versionFileRemotePath = remoteVersionFilePath projectNameAndVersion

Why Not

Combine with src/Lib.hs:1816:3
:warning: src/Lib.hs#L1398 - Found Reduce duplication
saveBinaryToLocalCache lCacheDir frameworkBinary
  (prefix </> remoteFrameworkUploadPath)
  fwn
  verbose
deleteFrameworkDirectory fVersion platform verbose
unzipBinary frameworkBinary fwn frameworkZipName verbose <*
  ifExists frameworkExecutablePath
    (makeExecutable frameworkExecutablePath)

Why Not

Combine with src/Lib.hs:1616:17
:warning: src/Lib.hs#L1430 - Found Reduce duplication
let symbolmapLoggingName
      = fwn <> "." <> bcsymbolmapNameFrom dwarfUUID
let bcsymbolmapZipName d = bcsymbolmapArchiveName d version
let localBcsymbolmapPathFrom d
      = platformBuildDirectory </> bcsymbolmapNameFrom d

Why Not

Combine with src/Lib.hs:1647:21
:warning: src/Lib.hs#L1440 - Found Reduce duplication
saveBinaryToLocalCache lCacheDir symbolmapBinary
  (prefix </> remoteBcSymbolmapUploadPathFromDwarf dwarfUUID)
  fwn
  verbose
deleteFile (localBcsymbolmapPathFrom dwarfUUID) verbose
unzipBinary symbolmapBinary symbolmapLoggingName
  (bcsymbolmapZipName dwarfUUID)
  verbose

Why Not

Combine with src/Lib.hs:1657:21
:warning: src/Lib.hs#L1475 - Found Reduce duplication
saveBinaryToLocalCache lCacheDir dSYMBinary
  (prefix </> remotedSYMUploadPath)
  dSYMName
  verbose
deleteDSYMDirectory fVersion platform verbose
unzipBinary dSYMBinary dSYMName dSYMZipName verbose

Why Not

Combine with src/Lib.hs:1691:17
:warning: src/Lib.hs#L1486 - Found Reduce duplication
frameworkZipName = frameworkArchiveName f version
remoteFrameworkUploadPath
  = remoteFrameworkPath platform reverseRomeMap f version
remoteBcSymbolmapUploadPathFromDwarf dwarfUUID
  = remoteBcsymbolmapPath dwarfUUID platform reverseRomeMap f version
dSYMZipName = dSYMArchiveName f version
remotedSYMUploadPath
  = remoteDsymPath platform reverseRomeMap f version
platformBuildDirectory
  = carthageArtifactsBuildDirectoryForPlatform platform f
dSYMName = fwn <> ".dSYM"
frameworkExecutablePath
  = frameworkBuildBundleForPlatform platform f </> fwn

Why Not

Combine with src/Lib.hs:1702:3
:warning: src/Lib.hs#L1528 - Found Use lambda-case
\ e ->
  case e of
      ErrorGettingDwarfUUIDs -> sayFunc $
                                  "Error: Cannot retrieve symbolmaps ids for " <> fwn
      (FailedDwarfUUIDs dwardUUIDsAndErrors) -> mapM_ (sayFunc . snd)
                                                  dwardUUIDsAndErrors

Why Not

\case
    ErrorGettingDwarfUUIDs -> sayFunc $
                                "Error: Cannot retrieve symbolmaps ids for " <> fwn
    (FailedDwarfUUIDs dwardUUIDsAndErrors) -> mapM_ (sayFunc . snd)
                                                dwardUUIDsAndErrors
:warning: src/Types/Commands.hs#L28 - Found Use newtype instead of data
data RomeUtilsPayload = RomeUtilsPayload{_subcommand ::
                                         RomeUtilsSubcommand}
                          deriving (Show, Eq)

Why Not

newtype RomeUtilsPayload = RomeUtilsPayload{_subcommand ::
                                            RomeUtilsSubcommand}
                             deriving (Show, Eq)
:warning: src/Utils.hs#L378 - Found Use tuple-section
\ f -> (f, g)

Why Not

(, g)
:warning: tests/Tests.hs#L138 - Found Use uncurry
\ (a, b) -> a || b

Why Not

uncurry (||)
:warning: tests/Tests.hs#L155 - Found Redundant bracket
(unProjectName (_projectName r)) ++ " = " ++ f

Why Not

unProjectName (_projectName r) ++ " = " ++ f
:warning: tests/Tests.hs#L156 - Found Redundant bracket
(map show) (_frameworks r)

Why Not

map show (_frameworks r)
:warning: tests/Tests.hs#L183 - Found Move brackets to avoid $
(length $ base `filterRomeFileEntriesByPlatforms` filteringValues)
  <= length base

Why Not

length (base `filterRomeFileEntriesByPlatforms` filteringValues) <=
  length base

Generated by :no_entry_sign: Danger

hlintBot avatar May 15 '19 17:05 hlintBot

@blender Thanks for this work, it will certainly save us a lot of time... would you be able to make a pre-release binary (or point me in the right direction) so I can test it and provide feedback?

ollieatkinson avatar May 31 '19 15:05 ollieatkinson

Hi,

I am not close to my computer at the moment. I will make a pre release on Monday.

If you feel adventurous you can try to compile yourself. Instructions are in the read me.

Sent from my iPhone

On 31 May 2019, at 17:21, Oliver Atkinson [email protected] wrote:

@blender Thanks for this work, it will certainly save us a lot of time... would you be able to make a pre-release binary with this so I can test it and provide feedback?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

tmspzz avatar May 31 '19 16:05 tmspzz

Thanks for the pointer @blender - I got the branch compiled, but it did not do what I expect. I assumed it would do the following:

  1. Check the Cartfile.resolved against the .version file
  2. If a match, no files relating to the framework would be requested (including bcsymbolmaps)
  3. If there's a version mismatch, or lack of a version file then it would download all of the files relating to the framework.

I was not seeing the above behaviour, all files relating to the frameworks with valid version files were still being downloaded.

However, there's a good chance I did not set it up correctly as I'm new to Rome.

edit:

That could be useful but still .dYSM and .bcsymbolmap are not checksummed in the .version file so they would need to be downloaded again.

I have just seen your comment on the original issue, maybe this is what I am seeing - is it safe to add an option which does not download those files if the version file is present?

ollieatkinson avatar Jun 01 '19 12:06 ollieatkinson

Did you run it with —optimistic ?

Sent from my iPhone

On 1 Jun 2019, at 14:04, Oliver Atkinson [email protected] wrote:

Thanks for the pointer @blender - I got the branch compiled, but it did not do what I expect. I assumed it would do the following:

Check the Cartfile.resolved against the .version file If a match, no files relating to the framework would be requested (including bcsymbolmaps) If there's a version mismatch, or lack of a version file then it would download all of the files relating to the framework. I was not seeing the above behaviour, all files relating to the frameworks with valid version files were still being downloaded.

However, there's a good chance I did not set it up correctly as I'm new to Rome.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

tmspzz avatar Jun 01 '19 13:06 tmspzz

Did you run it with —optimistic ?

Yes

ollieatkinson avatar Jun 01 '19 13:06 ollieatkinson

This needs tests if anyone looking for hacktoberfest issues wants to finish it

tmspzz avatar Oct 08 '19 12:10 tmspzz

Any progress on this PR?

vytautasgimbutas avatar Mar 04 '20 14:03 vytautasgimbutas

@vytautasgimbutas it needs to be adapted to the current code base and it needs integration tests. Do you want to give it a shot?

tmspzz avatar Mar 04 '20 14:03 tmspzz

I'd be very interested in this too. Is there an ETA for it?

felipe-azevedo avatar Mar 30 '20 17:03 felipe-azevedo

When this is made available, can you also please include the "optimistic" parameter in the fasltane rome plugin? Many thanks for this!

felipe-azevedo avatar Mar 30 '20 18:03 felipe-azevedo

Contributions are accepted @felipe-azevedo

ollieatkinson avatar Mar 30 '20 18:03 ollieatkinson