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

(PDK-1033) Use --unshallow when fetching a ref #247

Merged
merged 2 commits into from
Jun 20, 2018

Conversation

DavidS
Copy link
Contributor

@DavidS DavidS commented Jun 16, 2018

git does not fetch beyond the original depth in shallow repositories. This change uses --unshallow when a ref is specified, to avoid the issues shown in PDK-1033, namely not being able to switch from a shallow non-ref fixture clone to one with a ref to outside the original clone.

@DavidS DavidS added the bug label Jun 16, 2018
@DavidS
Copy link
Contributor Author

DavidS commented Jun 16, 2018

@cdenneen does this fix your issues?

@puppetcla
Copy link

CLA signed by all contributors.

@rodjek rodjek force-pushed the pdk-1033-git---unshallow branch 2 times, most recently from c41a3e3 to b39a501 Compare June 19, 2018 13:14
git does not fetch beyond the original depth in shallow repositories. This change uses `--unshallow` when a `ref` is specified, to avoid the issues shown in PDK-1033, namely not being able to switch from a shallow non-`ref` fixture clone to one with a `ref` to outside the original clone.
@cdenneen
Copy link

This is "better" but still not 100%

First run through I ran the spec_prep and got the following error:

Failed to clone git repository https://git.domain.com/puppet-external/voxpupuli-selinux.git into spec/fixtures/modules/selinux
/Users/cdenneen/src/gitlab/monitoring-puppet/.bundle/bundler/gems/puppetlabs_spec_helper-9b87f3f51345/lib/puppetlabs_spec_helper/tasks/fixtures.rb:121:in `clone_repo'
/Users/cdenneen/src/gitlab/monitoring-puppet/.bundle/bundler/gems/puppetlabs_spec_helper-9b87f3f51345/lib/puppetlabs_spec_helper/tasks/fixtures.rb:271:in `block (3 levels) in <top (required)>'
Tasks: TOP => spec_prep
(See full trace by running task with --trace)

Second run through Failed to clone no longer happen where as before this patch it would happen on every spec_prep. So I think @rodjek is on the right track since these are internal 'mirrors' or external repos for localization.

Also note getting the following warning on both runs

/Users/cdenneen/src/gitlab/monitoring-puppet/.bundle/bundler/gems/puppetlabs_spec_helper-9b87f3f51345/lib/puppetlabs_spec_helper/tasks/fixtures.rb:127: warning: conflicting chdir during another chdir block
/Users/cdenneen/src/gitlab/monitoring-puppet/.bundle/bundler/gems/puppetlabs_spec_helper-9b87f3f51345/lib/puppetlabs_spec_helper/tasks/fixtures.rb:127: warning: conflicting chdir during another chdir block

Another thing I've noticed is sometimes on subsequent spec_prep runs I get weird output like which has nothing to do with the symlinks specified in the .fixtures.yml, it's almost like it reads one of the fixtures fixtures and outputs but doesn't create symlink as it says... if I look in spec/fixtures/modules only symlink is the module I'm in or in this case profile and role:

I, [2018-06-19T10:40:51.298061 #66647]  INFO -- : Creating symlink from spec/fixtures/modules/selinux to /Users/cdenneen/src/gitlab/monitoring-puppet/spec/fixtures/modules/archive/spec/fixtures/modules/stdlib/spec/fixtures/modules/firewall/spec/fixtures/modules/ntp/spec/fixtures/modules/selinux

@puppetcla
Copy link

CLA signed by all contributors.

@DavidS
Copy link
Contributor Author

DavidS commented Jun 19, 2018

@cdenneen are you maybe seeing into https://tickets.puppetlabs.com/browse/PDK-1031 ?

@cdenneen
Copy link

@DavidS That definitely might be the "Symlink" weirdness.

Also did another spec_prep (never ran a spec_clean... these are just spec_prep over and over again) and it had a failure again... so had failure on selinux then good and then failure on snmp.

Failed to clone git repository https://git.domain.com/puppet-external/razorsedge-snmp.git into spec/fixtures/modules/snmp
/Users/cdenneen/src/gitlab/monitoring-puppet/.bundle/bundler/gems/puppetlabs_spec_helper-9b87f3f51345/lib/puppetlabs_spec_helper/tasks/fixtures.rb:121:in `clone_repo'
/Users/cdenneen/src/gitlab/monitoring-puppet/.bundle/bundler/gems/puppetlabs_spec_helper-9b87f3f51345/lib/puppetlabs_spec_helper/tasks/fixtures.rb:271:in `block (3 levels) in <top (required)>'
Tasks: TOP => spec_prep
(See full trace by running task with --trace)

@DavidS
Copy link
Contributor Author

DavidS commented Jun 19, 2018

That's weird. Do the clones actually look like clones, or did it just create an empty directory (which is enough for PSH to skip them).

@cdenneen
Copy link

cdenneen commented Jun 19, 2018

it was the threading... was hiding a fixture that was changed from fork back to upstream/ref and the fixture still had remote pointing at the fork (it never updated remote or recloned).

gitlab/monitoring-puppet - [dev●] » MAX_FIXTURE_THREAD_COUNT=1 be rake spec_prep
HEAD is now at 3721021 Merge pull request #317 from bastelfreak/rel230
HEAD is now at 37d996d Merge pull request #885 from pmcmaw/README_Changes
HEAD is now at e576069 Merge pull request #754 from pmcmaw/1.12.0_release
HEAD is now at c07a6b4 Merge pull request #429 from pmcmaw/releaseprep_7.0.0
HEAD is now at 6684f1c Merge pull request #168 from bastelfreak/rel080
HEAD is now at d15a58f [blacksmith] Bump version to 2.0.10
fatal: Could not parse object '44ab771c3aece999fbc752df3574b194eeb31563'.
HEAD is now at 49a1f04 Merge pull request #125 from pmcmaw/release_prep_pdk
HEAD is now at cbd7151 Update versions for 1.1.0 release.
HEAD is now at 477f1d8 Update versions for 6.1.0 release.
HEAD is now at 72e43bf Update versions for 3.9.0 release.
HEAD is now at 46c8b29 2.0.0 - Fix-up metadata and changelog
HEAD is now at 3d0e5ff Bump version to 1.7.1
HEAD is now at 6cbe87d new release v5.0.0
HEAD is now at 9c8bfd5 Readying for 1.3.0
HEAD is now at 65a3dc6 Prep 4.0.0 release
HEAD is now at 295de2b Support Puppet 5 (#25)
HEAD is now at 604680c Support Puppet 5 (#7)
HEAD is now at 012cd99 Provide the option to only update the live value (#15)
HEAD is now at b8a43a7 Merge pull request #263 from pmcmaw/release_prep_220
HEAD is now at b842daa Updating translations for readmes/README_ja_JP.md
HEAD is now at e6b1fbb Version increased for Puppet Forge update
HEAD is now at 1050381 Merge pull request #285 from HelenCampbell/release
HEAD is now at c8bfea4 Release 1.0.0
HEAD is now at 6102db86 Release prep
HEAD is now at 2f2529f Added a test for array of paths and exclusion parameter.
HEAD is now at e4a6b73 Update gemfile lock
HEAD is now at 9adccd0 Merge pull request #741 from pmcmaw/pdk_convert-release
HEAD is now at eccb86f Merge pull request #490 from HelenCampbell/rprep
HEAD is now at fa238da Merge pull request #677 from wyardley/release_8_1_0
HEAD is now at db56b6d Merge pull request #62 from queeno/update_apt_key
HEAD is now at bc434a7 Release 1.0.4
HEAD is now at 07c4259 Update CHangelog
HEAD is now at 28c3c00 Release 0.9.8
HEAD is now at aa0f925 Release v2.50.0
HEAD is now at a7a67a4 Increment version to 2.4.0
HEAD is now at adfc288 v1.1.3
HEAD is now at 0f06a59 Merge pull request #67 from bastelfreak/rel200

Once i deleted the fixture module it worked fine. Also subsequent run works clean as well. So threading was hiding the underlying issue.

gitlab/monitoring-puppet - [dev●] » MAX_FIXTURE_THREAD_COUNT=1 be rake spec_prep
HEAD is now at 3721021 Merge pull request #317 from bastelfreak/rel230
HEAD is now at 37d996d Merge pull request #885 from pmcmaw/README_Changes
HEAD is now at e576069 Merge pull request #754 from pmcmaw/1.12.0_release
HEAD is now at c07a6b4 Merge pull request #429 from pmcmaw/releaseprep_7.0.0
HEAD is now at 6684f1c Merge pull request #168 from bastelfreak/rel080
HEAD is now at d15a58f [blacksmith] Bump version to 2.0.10
Cloning into 'spec/fixtures/modules/postfix'...
remote: Counting objects: 2122, done.
remote: Compressing objects: 100% (913/913), done.
remote: Total 2122 (delta 1242), reused 2016 (delta 1163)
Receiving objects: 100% (2122/2122), 392.85 KiB | 7.14 MiB/s, done.
Resolving deltas: 100% (1242/1242), done.
HEAD is now at 44ab771 Relax permissions
HEAD is now at 49a1f04 Merge pull request #125 from pmcmaw/release_prep_pdk
HEAD is now at cbd7151 Update versions for 1.1.0 release.
HEAD is now at 477f1d8 Update versions for 6.1.0 release.
HEAD is now at 72e43bf Update versions for 3.9.0 release.
HEAD is now at 46c8b29 2.0.0 - Fix-up metadata and changelog
HEAD is now at 3d0e5ff Bump version to 1.7.1
HEAD is now at 6cbe87d new release v5.0.0
HEAD is now at 9c8bfd5 Readying for 1.3.0
HEAD is now at 65a3dc6 Prep 4.0.0 release
HEAD is now at 295de2b Support Puppet 5 (#25)
HEAD is now at 604680c Support Puppet 5 (#7)
HEAD is now at 012cd99 Provide the option to only update the live value (#15)
HEAD is now at b8a43a7 Merge pull request #263 from pmcmaw/release_prep_220
HEAD is now at b842daa Updating translations for readmes/README_ja_JP.md
HEAD is now at e6b1fbb Version increased for Puppet Forge update
HEAD is now at 1050381 Merge pull request #285 from HelenCampbell/release
HEAD is now at c8bfea4 Release 1.0.0
HEAD is now at 6102db86 Release prep
HEAD is now at 2f2529f Added a test for array of paths and exclusion parameter.
HEAD is now at e4a6b73 Update gemfile lock
HEAD is now at 9adccd0 Merge pull request #741 from pmcmaw/pdk_convert-release
HEAD is now at eccb86f Merge pull request #490 from HelenCampbell/rprep
HEAD is now at fa238da Merge pull request #677 from wyardley/release_8_1_0
HEAD is now at db56b6d Merge pull request #62 from queeno/update_apt_key
HEAD is now at bc434a7 Release 1.0.4
HEAD is now at 07c4259 Update CHangelog
HEAD is now at 28c3c00 Release 0.9.8
HEAD is now at aa0f925 Release v2.50.0
HEAD is now at a7a67a4 Increment version to 2.4.0
HEAD is now at adfc288 v1.1.3
HEAD is now at 0f06a59 Merge pull request #67 from bastelfreak/rel200

gitlab/monitoring-puppet - [dev●] » MAX_FIXTURE_THREAD_COUNT=1 be rake spec_prep
HEAD is now at 3721021 Merge pull request #317 from bastelfreak/rel230
HEAD is now at 37d996d Merge pull request #885 from pmcmaw/README_Changes
HEAD is now at e576069 Merge pull request #754 from pmcmaw/1.12.0_release
HEAD is now at c07a6b4 Merge pull request #429 from pmcmaw/releaseprep_7.0.0
HEAD is now at 6684f1c Merge pull request #168 from bastelfreak/rel080
HEAD is now at d15a58f [blacksmith] Bump version to 2.0.10
HEAD is now at 44ab771 Relax permissions
HEAD is now at 49a1f04 Merge pull request #125 from pmcmaw/release_prep_pdk
HEAD is now at cbd7151 Update versions for 1.1.0 release.
HEAD is now at 477f1d8 Update versions for 6.1.0 release.
HEAD is now at 72e43bf Update versions for 3.9.0 release.
HEAD is now at 46c8b29 2.0.0 - Fix-up metadata and changelog
HEAD is now at 3d0e5ff Bump version to 1.7.1
HEAD is now at 6cbe87d new release v5.0.0
HEAD is now at 9c8bfd5 Readying for 1.3.0
HEAD is now at 65a3dc6 Prep 4.0.0 release
HEAD is now at 295de2b Support Puppet 5 (#25)
HEAD is now at 604680c Support Puppet 5 (#7)
HEAD is now at 012cd99 Provide the option to only update the live value (#15)
HEAD is now at b8a43a7 Merge pull request #263 from pmcmaw/release_prep_220
HEAD is now at b842daa Updating translations for readmes/README_ja_JP.md
HEAD is now at e6b1fbb Version increased for Puppet Forge update
HEAD is now at 1050381 Merge pull request #285 from HelenCampbell/release
HEAD is now at c8bfea4 Release 1.0.0
HEAD is now at 6102db86 Release prep
HEAD is now at 2f2529f Added a test for array of paths and exclusion parameter.
HEAD is now at e4a6b73 Update gemfile lock
HEAD is now at 9adccd0 Merge pull request #741 from pmcmaw/pdk_convert-release
HEAD is now at eccb86f Merge pull request #490 from HelenCampbell/rprep
HEAD is now at fa238da Merge pull request #677 from wyardley/release_8_1_0
HEAD is now at db56b6d Merge pull request #62 from queeno/update_apt_key
HEAD is now at bc434a7 Release 1.0.4
HEAD is now at 07c4259 Update CHangelog
HEAD is now at 28c3c00 Release 0.9.8
HEAD is now at aa0f925 Release v2.50.0
HEAD is now at a7a67a4 Increment version to 2.4.0
HEAD is now at adfc288 v1.1.3
HEAD is now at 0f06a59 Merge pull request #67 from bastelfreak/rel200

@codecov-io
Copy link

codecov-io commented Jun 20, 2018

Codecov Report

Merging #247 into master will decrease coverage by 0.21%.
The diff coverage is 21.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #247      +/-   ##
==========================================
- Coverage   39.37%   39.16%   -0.22%     
==========================================
  Files          10       10              
  Lines         706      720      +14     
==========================================
+ Hits          278      282       +4     
- Misses        428      438      +10
Impacted Files Coverage Δ
lib/puppetlabs_spec_helper/tasks/fixtures.rb 28.98% <21.05%> (-0.04%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3e058b...4e8e992. Read the comment docs.

@rodjek
Copy link
Contributor

rodjek commented Jun 20, 2018

The threadsafe issue is being handled in #249

Copy link
Contributor Author

@DavidS DavidS left a comment

Choose a reason for hiding this comment

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

👍

@DavidS DavidS merged commit cf972bb into master Jun 20, 2018
@DavidS DavidS deleted the pdk-1033-git---unshallow branch June 20, 2018 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants