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

Merge 'mapbox-gl-test-suite' repo without preserving git history #3834

Merged
merged 5 commits into from
Jan 10, 2017

Conversation

lucaswoj
Copy link
Contributor

@lucaswoj lucaswoj commented Dec 19, 2016

This PR merges mapbox-gl-test-suite into this repo. We do not preserve git history to reduce repo size. We introduce git-lfs to reduce repo size.

The merge is pretty naive, dropping mapbox-gl-test-suite into a test/integration folder unmodified. More extensive merging of fixture libraries and code is planned for a future PR.

fixes #3795

@lucaswoj lucaswoj force-pushed the merge-test-suite branch 2 times, most recently from e3acc7b to 965bce4 Compare December 20, 2016 00:08
@jfirebaugh
Copy link
Contributor

Getting some errors when trying to use this in mapbox-gl-native:

~/Development/mapbox-gl-native $ npm install
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: Downloading test/fixtures/mbsv5-6-18-23.vector.pbf (71.19 KB)
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: Error downloading object: test/fixtures/mbsv5-6-18-23.vector.pbf (d1b2db99907dbfd9747dc88e8ef5b91974b21222f6dde73f5507bec7e957545b)
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: 
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: Errors logged to /private/var/folders/vp/d4cnkmkx3pz5tj36vw4g5hbr0000gn/T/npm-84441-263c2371/git-cache-723ed10f/e68e3a6d5035d77f21dc15256369866cac7bfeab/.git/lfs/objects/logs/20161219T160701.103130306.log
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: Use `git lfs logs last` to view the log.
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: error: external filter git-lfs smudge %f failed 2
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: error: external filter git-lfs smudge %f failed
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: fatal: test/fixtures/mbsv5-6-18-23.vector.pbf: smudge filter lfs failed
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: 
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: Downloading test/fixtures/mbsv5-6-18-23.vector.pbf (71.19 KB)
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: Error downloading object: test/fixtures/mbsv5-6-18-23.vector.pbf (d1b2db99907dbfd9747dc88e8ef5b91974b21222f6dde73f5507bec7e957545b)
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: 
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: Errors logged to /private/var/folders/vp/d4cnkmkx3pz5tj36vw4g5hbr0000gn/T/npm-84441-263c2371/git-cache-99b6f612/e68e3a6d5035d77f21dc15256369866cac7bfeab/.git/lfs/objects/logs/20161219T160808.428575012.log
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: Use `git lfs logs last` to view the log.
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: error: external filter git-lfs smudge %f failed 2
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: error: external filter git-lfs smudge %f failed
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: fatal: test/fixtures/mbsv5-6-18-23.vector.pbf: smudge filter lfs failed
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: 
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: Downloading test/fixtures/mbsv5-6-18-23.vector.pbf (71.19 KB)
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: Error downloading object: test/fixtures/mbsv5-6-18-23.vector.pbf (d1b2db99907dbfd9747dc88e8ef5b91974b21222f6dde73f5507bec7e957545b)
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: 
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: Errors logged to /private/var/folders/vp/d4cnkmkx3pz5tj36vw4g5hbr0000gn/T/npm-84441-263c2371/git-cache-9e6dd10d/e68e3a6d5035d77f21dc15256369866cac7bfeab/.git/lfs/objects/logs/20161219T160908.323343201.log
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: Use `git lfs logs last` to view the log.
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: error: external filter git-lfs smudge %f failed 2
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: error: external filter git-lfs smudge %f failed
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: fatal: test/fixtures/mbsv5-6-18-23.vector.pbf: smudge filter lfs failed
npm ERR! git checkout e68e3a6d5035d77f21dc15256369866cac7bfeab: 
npm ERR! Darwin 15.6.0
npm ERR! argv "/usr/local/Cellar/node@6/6.9.1/bin/node" "/usr/local/bin/npm" "install"
npm ERR! node v6.9.1
npm ERR! npm  v3.10.8
npm ERR! code 128

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Dec 20, 2016

I have a couple theories about the errors you're seeing. They could be caused by:

  • not having git-lfs installed on your machine
  • npm not playing nice with git lfs
  • me force pushing a few small fixes to this PR
  • me configuring git lfs incorrectly

EDIT I'm seeing some odd things on CI too https://circleci.com/gh/mapbox/mapbox-gl-js/6368

@lucaswoj
Copy link
Contributor Author

@lucaswoj lucaswoj force-pushed the merge-test-suite branch 2 times, most recently from 63b509d to 7db48cc Compare December 20, 2016 17:49
@lucaswoj
Copy link
Contributor Author

I think I've found workable solutions to the CI and npm install challenges. Can you try again @jfirebaugh?

@lucaswoj
Copy link
Contributor Author

Word of caution: opening this PR's "Files changed" tab on github crashes Chrome 😬

@jfirebaugh
Copy link
Contributor

Can you remove the actual.pngs? They shouldn't be checked in.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Dec 20, 2016

Good catch @jfirebaugh. It looks like I updated .gitignore after adding the files to git. Fixed in the latest commit. (there were some diff.pngs and index.htmls checked in too). This should be a boon to check out time.

@jfirebaugh
Copy link
Contributor

In my tests, git lfs is slower than our current mapbox-gl-test-suite setup on the cases where we currently suffer: fresh clones.

With git 2.11.0 and git-lfs 1.5.3, the initial checkout of this branch took 21m46.766s. It's true that git lfs caches the downloaded files, but this is a cost that anyone cloning the repository for the first time will have to pay. That includes installation of a git npm dependency, which does git clone --mirror.

In mapbox-gl-native, after removing the mapbox-gl-test-suite dependency and updating the mapbox-gl dependency to point at 68feb9c, a fresh npm install took a similar amount of time -- 22m53.584s. A non-fresh npm install after bumping the SHA of the mapbox-gl dev dependency from 68feb9c to 65a49c3 (a noop commit) took 17m56.164s. For comparison, equivalent operations with the current setup take about 14m.

TL;DR, with npm, any time a git dependency changes, it's a fresh git clone, and that's slower with git lfs than cloning the entire history of mapbox-gl-test-suite. It doesn't seem like this workload is a good fit for git lfs.

@lucaswoj
Copy link
Contributor Author

I agree. I'll strip out lfs in a subsequent commit.

cc @mourner

@lucaswoj lucaswoj force-pushed the merge-test-suite branch 3 times, most recently from 597a0a8 to 3f33d6b Compare December 21, 2016 14:35
@lucaswoj lucaswoj changed the title Merged 'mapbox-gl-test-suite' repo without preserving git history and adding lfs Merged 'mapbox-gl-test-suite' repo without preserving git history Dec 21, 2016
@lucaswoj
Copy link
Contributor Author

Ready to 🚢?

@jfirebaugh
Copy link
Contributor

Hold on this one please. I need to make sure it integrates cleanly into mapbox-gl-native.

@lucaswoj
Copy link
Contributor Author

No problem 👍. Any idea how adding expected.png files and fixtures to .npmignore would effect GL Native?

@jfirebaugh
Copy link
Contributor

It looks like that'll be an issue actually -- after installing as a dev dependency in GL native, node_modules/mapbox-gl doesn't have a test directory at all.

@lucaswoj lucaswoj changed the title Merged 'mapbox-gl-test-suite' repo without preserving git history Merge 'mapbox-gl-test-suite' repo without preserving git history Dec 22, 2016
@jfirebaugh
Copy link
Contributor

jfirebaugh commented Jan 2, 2017

Looking promising downstream in gl-native: mapbox/mapbox-gl-native#7531. @lucaswoj could you update the branch here with the latest from test-suite?

Lucas Wojciechowski added 2 commits January 9, 2017 14:50
@lucaswoj
Copy link
Contributor Author

lucaswoj commented Jan 9, 2017

Updated to latest test suite SHA @jfirebaugh (mapbox/mapbox-gl-test-suite@9ab1943)

@lucaswoj lucaswoj merged commit 8780088 into master Jan 10, 2017
@lucaswoj
Copy link
Contributor Author

Merging per chat with @jfirebaugh 🎉

@lucaswoj lucaswoj deleted the merge-test-suite branch January 10, 2017 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merge the "mapbox-gl-test-suite" repo into this repo
2 participants