-
Notifications
You must be signed in to change notification settings - Fork 184
CI improvements #525
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
CI improvements #525
Changes from 10 commits
7e8cb9f
7b33fe3
4da29c5
fc83d92
be92579
f7e3656
55c342d
dd3fd22
00480bc
43d252c
741d1c5
9d12f78
2a071ad
05e66f2
1f2bea0
bb5c964
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,14 +2,15 @@ version: 2.1 | |
|
|
||
| orbs: | ||
| win: circleci/windows@4.1 | ||
| ruby: circleci/ruby@2.0.0 | ||
|
|
||
| jobs: | ||
| test_linux: | ||
| parameters: | ||
| ruby_version: | ||
| description: 'version tag for the cimg/ruby container' | ||
| type: string | ||
|
|
||
| machine: | ||
| image: ubuntu-2004:current | ||
|
|
||
|
|
@@ -19,11 +20,13 @@ jobs: | |
|
|
||
| - run: | ||
| name: start docker-compose build environment | ||
| command: | | ||
| command: | | ||
| sudo ./test/bin/setup_volume_permissions.sh | ||
| docker-compose up -d | ||
| echo "Waiting for containers to start..." | ||
| sleep 10 | ||
| environment: | ||
| RUBY_VERSION: << parameters.ruby_version >> | ||
|
|
||
| - run: | ||
| name: install sql prereqs | ||
|
|
@@ -58,7 +61,10 @@ jobs: | |
| - run: | ||
| name: test gem | ||
| command: | | ||
| docker exec cimg_ruby bash -c 'bundle exec rake test' | ||
| docker exec cimg_ruby bash -c 'bundle exec rake test' | ||
|
|
||
| - store_test_results: | ||
| path: test/reports | ||
|
|
||
| test_windows: | ||
| parameters: | ||
|
|
@@ -108,11 +114,10 @@ jobs: | |
| rm .\ruby-setup.exe | ||
|
|
||
| - run: | ||
| name: update build env | ||
| name: install bundler | ||
| command: | | ||
| $Env:PATH = "C:\\Ruby<< parameters.ruby_version >>-x64\\bin;$Env:PATH" | ||
| ridk install 2 | ||
| gem install bundler | ||
| gem install bundler -v 2.3.26 | ||
|
|
||
| - checkout | ||
|
|
||
|
|
@@ -127,57 +132,140 @@ jobs: | |
| name: bundle install gems | ||
| command: | | ||
| $Env:PATH = "C:\\Ruby<< parameters.ruby_version >>-x64\\bin;$Env:PATH" | ||
| bundle install | ||
| bundle install --path vendor/bundle | ||
|
|
||
| - save_cache: | ||
| name: save gem cache | ||
| paths: | ||
| - ./vendor/bundle | ||
| key: v1-bundle-<< parameters.ruby_version >>-{{ .Branch }}-{{ checksum "tiny_tds.gemspec" }} | ||
|
|
||
| - restore_cache: | ||
| name: restore cross-compiled code | ||
| keys: | ||
| - compiled-<< pipeline.git.revision >> | ||
|
|
||
| - run: | ||
| name: build openssl | ||
| no_output_timeout: 30m | ||
| name: restore cross-compiled code | ||
| command: | | ||
| $Env:PATH = "C:\\Ruby<< parameters.ruby_version >>-x64\\bin;$Env:PATH" | ||
| bundle exec rake ports:openssl | ||
|
|
||
| $rubyArchitecture = (ruby -e 'puts RUBY_PLATFORM').Trim() | ||
| $source = "C:\home\circleci\project\tmp\$rubyArchitecture\stage\lib\tiny_tds" | ||
| $destination = ".\lib\tiny_tds" | ||
| Get-ChildItem $source -Recurse -Exclude "*.rb" | Copy-Item -Destination {Join-Path $destination $_.FullName.Substring($source.length)} | ||
|
|
||
| - restore_cache: | ||
| name: restore mssql installation file | ||
| key: downloads-{{ checksum "test/bin/install-mssql.ps1" }} | ||
|
Comment on lines
+167
to
+169
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you clarify the benefit of caching this installation script? It's just another source file in the repo, so it should already be available on disk in the job step.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't cache the PowerShell script, but the Potential side-effect is that if you change something in the script, it'll re-download the installation file. But it was also continue to use the cache even if Microsoft updates the SQL installation file on their side. I wouldn't say it's too important that the CI uses the latest version and rather take the 2 minute speed-up from caching the file.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Oh yeah... duh. |
||
|
|
||
| - run: | ||
| name: build libiconv | ||
| no_output_timeout: 30m | ||
| name: setup mssql | ||
| command: | | ||
| $Env:PATH = "C:\\Ruby<< parameters.ruby_version >>-x64\\bin;$Env:PATH" | ||
| bundle exec rake ports:libiconv | ||
| .\test\bin\install-mssql.ps1 | ||
|
|
||
| - save_cache: | ||
| name: save downloads cache | ||
| paths: | ||
| - C:\Downloads | ||
| key: downloads-{{ checksum "test/bin/install-mssql.ps1" }} | ||
|
|
||
| - run: | ||
| name: build freetds | ||
| no_output_timeout: 30m | ||
| name: install toxiproxy-server | ||
| command: | | ||
| $Env:PATH = "C:\\Ruby<< parameters.ruby_version >>-x64\\bin;$Env:PATH" | ||
| bundle exec rake ports:freetds | ||
| choco install toxiproxy-server --version=2.5.0 -y | ||
| Start-Process toxiproxy-server | ||
|
|
||
| - restore_cache: | ||
| name: restore ports cache | ||
| keys: | ||
| - ports-libiconv115-openssl111s-freetds1124 | ||
|
|
||
| - run: | ||
| name: build gem | ||
| no_output_timeout: 30m | ||
| name: copy ports cache into project | ||
| command: | | ||
| Move-Item C:\home\circleci\project\ports . | ||
|
|
||
| - run: | ||
| name: test gem | ||
| command: | | ||
| $Env:PATH = "C:\\Ruby<< parameters.ruby_version >>-x64\\bin;$Env:PATH" | ||
| bundle exec rake ports | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ha, oops |
||
| bundle exec rake test | ||
| environment: | ||
| TOXIPROXY_HOST: "localhost" | ||
|
|
||
| - store_test_results: | ||
| path: test/reports | ||
|
|
||
| cross_compile_gem: | ||
| machine: | ||
| image: ubuntu-2004:current | ||
|
|
||
| steps: | ||
| - ruby/install: | ||
| version: '2.7' | ||
| - checkout | ||
| - restore_cache: | ||
| name: restore gem cache | ||
| keys: | ||
| - v1-bundle-{{ .Branch }}-{{ checksum "tiny_tds.gemspec" }} | ||
| - v1-bundle-{{ .Branch }}- | ||
| - v1-bundle- | ||
|
|
||
| - run: | ||
| name: bundle install gems | ||
| command: | | ||
| bundle install --path vendor/bundle | ||
|
|
||
| - save_cache: | ||
| name: save gem cache | ||
| paths: | ||
| - ./vendor/bundle | ||
| key: v1-bundle-{{ .Branch }}-{{ checksum "tiny_tds.gemspec" }} | ||
|
|
||
| - restore_cache: | ||
| name: restore ports cache | ||
| keys: | ||
| - ports-libiconv115-openssl111s-freetds1124 | ||
|
|
||
| - run: | ||
| name: Build gems | ||
| command: | | ||
| bundle exec rake gem | ||
| bundle exec rake gem:native | ||
|
|
||
| # Move gem files to a separate directory, as CircleCI artifacts don't support wildcards for paths | ||
| mkdir pkg_artifacts | ||
| mv pkg/*.gem pkg_artifacts | ||
|
|
||
| - store_artifacts: | ||
| path: pkg_artifacts | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting... here's some other options for discussion:
I'm not fully opposed to the current approach, although would you consider calling the new subfolder "gems" since that's what'll go in (rather than pkg_artifacts)? Also, I recommend having the mkdir/move be a separate job step. Generally my preference is to have more single-purpose job steps rather than job steps with multiple commands.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I implemented your suggestions with 741d1c5. |
||
|
|
||
| - save_cache: | ||
| name: save ports cache | ||
| paths: | ||
| - ./ports | ||
| key: ports-libiconv115-openssl111s-freetds1124 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm trying to brainstorm alternate approaches here... even something like Also, can you clarify why it's necessary to switch from one job step per ports to compiling them all in one step? I preferred the version where we were able to see each of those individually. If it was just for caching, I assume we could still do restore cache, iconv, freetds, openssl, save cache, since each one of those individual steps would (or wouldn't) benefit from the restored cache, and would all be present by the time save_cache ran.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 9d12f78 implements what you suggest with file the base the cache on. Let me know what you think about the code changes. I didn't want to repeat the library names again, that was my main concern behind the changes.
You kind of mentioned my intention behind this below:
I'll give a more detailed answer in that conversation. |
||
|
|
||
| - save_cache: | ||
| name: save cross-compiled-code | ||
| paths: | ||
| - ./tmp | ||
| key: compiled-<< pipeline.git.revision >> | ||
|
|
||
| workflows: | ||
| test_supported_ruby_versions: | ||
| jobs: | ||
| - test_linux: | ||
| matrix: | ||
| - cross_compile_gem | ||
| - test_windows: | ||
| requires: | ||
| - cross_compile_gem | ||
| matrix: &ruby_versions | ||
| parameters: | ||
| ruby_version: | ||
| - '2.4' | ||
| - '2.5' | ||
| - '2.6' | ||
| - '2.7' | ||
|
|
||
| - test_windows: | ||
| matrix: | ||
| parameters: | ||
| ruby_version: | ||
| - '2.5' | ||
| - '2.6' | ||
| - '2.7' | ||
| - test_linux: | ||
| matrix: *ruby_versions | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,3 +18,4 @@ misc | |
| /exe/* | ||
| /ports/* | ||
| !/ports/patches/ | ||
| test/reports | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,2 @@ | ||
| source 'https://rubygems.org' | ||
| gemspec | ||
|
|
||
| group :development do | ||
| end | ||
|
|
||
| group :test do | ||
| gem 'minitest' | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,13 @@ | ||
| # encoding: UTF-8 | ||
|
|
||
| desc 'Build the windows binary gems per rake-compiler-dock' | ||
| task 'gem:windows' => ['ports:cross'] do | ||
| desc 'Build the native binary gems using rake-compiler-dock' | ||
| task 'gem:native' => ['ports:cross'] do | ||
| require 'rake_compiler_dock' | ||
|
|
||
| # make sure to install our bundle | ||
| build = ['bundle'] | ||
| sh "bundle package --all" # Avoid repeated downloads of gems by using gem files from the host. | ||
|
|
||
| # and finally build the native gem | ||
| build << 'rake cross native gem RUBY_CC_VERSION=2.7.0:2.6.0:2.5.0:2.4.0 CFLAGS="-Wall" MAKE="make -j`nproc`"' | ||
|
|
||
| RakeCompilerDock.sh build.join(' && ') | ||
| GEM_PLATFORM_HOSTS.keys.each do |plat| | ||
| RakeCompilerDock.sh "bundle --local && RUBY_CC_VERSION=#{RUBY_CC_VERSION} rake native:#{plat} gem", platform: plat | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| $ProgressPreference = 'SilentlyContinue' | ||
|
|
||
| if (-not(Test-path "C:\Downloads")) | ||
| { | ||
| mkdir "C:\Downloads" | ||
| } | ||
|
|
||
| $sqlInstallationFile = "C:\Downloads\sqlexpress.exe" | ||
| if (-not(Test-path $sqlInstallationFile -PathType leaf)) | ||
| { | ||
| Write-Host "Downloading SQL Express ..." | ||
| Invoke-WebRequest -Uri "https://go.microsoft.com/fwlink/?linkid=829176" -OutFile "C:\Downloads\sqlexpress.exe" | ||
| } | ||
|
|
||
| Write-Host "Installing SQL Express ..." | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I definitely agree it'd be great to test on Windows (especially since the job name is already "test windows"... oops... 🤦♂️). However, installing SQL Server is SUCH a headache. Are we able to use Docker for this? I don't think the Windows container is available any longer, so we'd need to use the Linux image, which ought to be possible by connecting to it via setup_remote_docker, I think
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah the Windows container are no longer available. But also Windows containers were quite a pain to set up, I can see why it was easier for Microsoft to just push WSL2 😁
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That said, I use a similar version of this script at work and has been working without any changes for more than a year now. I also use it for SQL server 2019 and 2022, so in case the pipeline should be extended at some point to test across different SQL server versions, it should be possible with little modification. |
||
| Start-Process -Wait -FilePath "C:\Downloads\sqlexpress.exe" -ArgumentList /qs, /x:"C:\Downloads\setup" | ||
| C:\Downloads\setup\setup.exe /q /ACTION=Install /INSTANCENAME=SQLEXPRESS /FEATURES=SQLEngine /UPDATEENABLED=0 /SQLSVCACCOUNT='NT AUTHORITY\System' /SQLSYSADMINACCOUNTS='BUILTIN\ADMINISTRATORS' /TCPENABLED=1 /NPENABLED=0 /IACCEPTSQLSERVERLICENSETERMS | ||
|
|
||
| Write-Host "Configuring SQL Express ..." | ||
| stop-service MSSQL`$SQLEXPRESS | ||
| set-itemproperty -path 'HKLM:\software\microsoft\microsoft sql server\mssql14.SQLEXPRESS\mssqlserver\supersocketnetlib\tcp\ipall' -name tcpdynamicports -value '' | ||
| set-itemproperty -path 'HKLM:\software\microsoft\microsoft sql server\mssql14.SQLEXPRESS\mssqlserver\supersocketnetlib\tcp\ipall' -name tcpport -value 1433 | ||
| set-itemproperty -path 'HKLM:\software\microsoft\microsoft sql server\mssql14.SQLEXPRESS\mssqlserver\' -name LoginMode -value 2 | ||
|
|
||
| Write-Host "Starting SQL Express ..." | ||
| start-service MSSQL`$SQLEXPRESS | ||
|
|
||
| Write-Host "Configuring MSSQL for TinyTDS ..." | ||
| & sqlcmd -Q "CREATE DATABASE [tinytdstest];" | ||
| & sqlcmd -Q "CREATE LOGIN [tinytds] WITH PASSWORD = '', CHECK_POLICY = OFF, DEFAULT_DATABASE = [tinytdstest];" | ||
| & sqlcmd -Q "USE [tinytdstest]; CREATE USER [tinytds] FOR LOGIN [tinytds]; EXEC sp_addrolemember N'db_owner', N'tinytds';" | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. So the Windows job has a
requiresdeclaration on the precompile job. So in case that one fails, the Windows jobs wouldn't even start.Regarding cache, I did some more research here and looks like workspaces are the better data store, as they are meant to pass build artifacts down in CI. Didn't know about that before, but it's implement now with 2a071ad. This also gets rid of the absolute paths.