From b1552f8e9b32abd985fab2c5b36d81dc3dd9e05a Mon Sep 17 00:00:00 2001 From: Paulo Castro Date: Wed, 3 Jun 2020 18:03:44 +0100 Subject: [PATCH 1/2] v12 preparations: Fix dockerignore tests on Windows Change-type: patch --- tests/commands/push.spec.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/commands/push.spec.ts b/tests/commands/push.spec.ts index 390bca74..f7bf8d93 100644 --- a/tests/commands/push.spec.ts +++ b/tests/commands/push.spec.ts @@ -367,7 +367,8 @@ describe('balena push', function() { 'src/src-b.txt': { fileSize: 5, type: 'file' }, 'symlink-a.txt': { fileSize: 5, type: 'file' }, }; - if (isWindows) { + const noV12W = isWindows && !isV12(); + if (noV12W) { // this test uses the old tarDirectory implementation, which uses // the zeit/dockerignore library that has bugs on Windows expectedFiles['src/src-a.txt'] = { fileSize: 5, type: 'file' }; @@ -378,7 +379,7 @@ describe('balena push', function() { path.join(builderResponsePath, responseFilename), 'utf8', ); - const expectedResponseLines = isWindows + const expectedResponseLines = noV12W ? [ '[Warn] Using file ignore patterns from:', `[Warn] ${path.join(projectPath, '.dockerignore')}`, From 1569915fae760eb68da00dfa18f48479d5340a96 Mon Sep 17 00:00:00 2001 From: Scott Lowe Date: Wed, 3 Jun 2020 15:41:12 +0200 Subject: [PATCH 2/2] v12 preparations: Add feature switch for default eol-converson Change-type: patch Connects-to: #1770 --- lib/actions/build.js | 6 +-- lib/actions/deploy.js | 13 +---- lib/actions/push.ts | 22 +++++++-- lib/utils/compose-types.d.ts | 1 + lib/utils/compose.js | 15 +++++- lib/utils/device/deploy.ts | 1 + lib/utils/eol-conversion.ts | 5 +- tests/commands/build.spec.ts | 81 ++++++++++++++++++++------------ tests/commands/deploy.spec.ts | 30 +++++++----- tests/commands/push.spec.ts | 61 +++++++++++++++--------- tests/utils/tarDirectory.spec.ts | 4 ++ 11 files changed, 156 insertions(+), 83 deletions(-) diff --git a/lib/actions/build.js b/lib/actions/build.js index 139345a3..a9b96f57 100644 --- a/lib/actions/build.js +++ b/lib/actions/build.js @@ -60,7 +60,7 @@ const buildProject = function(docker, logger, composeOpts, opts) { opts.buildEmulated, opts.buildOpts, composeOpts.inlineLogs, - opts.convertEol, + composeOpts.convertEol, composeOpts.dockerfilePath, composeOpts.nogitignore, ); @@ -152,9 +152,6 @@ Examples: } delete params.source; - options.convertEol = options['convert-eol'] || false; - delete options['convert-eol']; - const { application, arch, deviceType } = options; return Promise.try(function() { @@ -203,7 +200,6 @@ Examples: deviceType: resolvedDeviceType, buildEmulated: !!options.emulated, buildOpts, - convertEol: options.convertEol, }), ); }); diff --git a/lib/actions/deploy.js b/lib/actions/deploy.js index 4b42585e..52e6e111 100644 --- a/lib/actions/deploy.js +++ b/lib/actions/deploy.js @@ -97,7 +97,7 @@ const deployProject = function(docker, logger, composeOpts, opts) { opts.buildEmulated, opts.buildOpts, composeOpts.inlineLogs, - opts.convertEol, + composeOpts.convertEol, composeOpts.dockerfilePath, composeOpts.nogitignore, ) @@ -263,16 +263,6 @@ Examples: appName = appName_raw || appName || options.application; delete options.application; - options.convertEol = options['convert-eol'] || false; - delete options['convert-eol']; - if (options.convertEol && !options.build) { - return Promise.reject( - new ExpectedError( - 'The --eol-conversion flag is only valid with --build.', - ), - ); - } - return Promise.try(function() { if (appName == null) { throw new ExpectedError( @@ -320,7 +310,6 @@ Examples: shouldUploadLogs: !options.nologupload, buildEmulated: !!options.emulated, buildOpts, - convertEol: options.convertEol, }), ); }); diff --git a/lib/actions/push.ts b/lib/actions/push.ts index 9be77c2f..809f458f 100644 --- a/lib/actions/push.ts +++ b/lib/actions/push.ts @@ -117,6 +117,7 @@ export const push: CommandDefinition< system?: boolean; env?: string | string[]; 'convert-eol'?: boolean; + 'noconvert-eol'?: boolean; } > = { signature: 'push ', @@ -258,11 +259,23 @@ export const push: CommandDefinition< { signature: 'convert-eol', alias: 'l', - description: stripIndent` + description: isV12() + ? 'No-op and deprecated since balena CLI v12.0.0' + : stripIndent` On Windows only, convert line endings from CRLF (Windows format) to LF (Unix format). Source files are not modified.`, boolean: true, }, + ...(isV12() + ? [ + { + signature: 'noconvert-eol', + description: + "Don't convert line endings from CRLF (Windows format) to LF (Unix format).", + boolean: true, + }, + ] + : []), { signature: 'nogitignore', alias: 'G', @@ -307,6 +320,9 @@ export const push: CommandDefinition< ); const nogitignore = !!options.nogitignore || isV12(); + const convertEol = isV12() + ? !options['noconvert-eol'] + : !!options['convert-eol']; const buildTarget = getBuildTarget(appOrDevice); switch (buildTarget) { @@ -346,7 +362,7 @@ export const push: CommandDefinition< nocache: options.nocache || false, registrySecrets, headless: options.detached || false, - convertEol: options['convert-eol'] || false, + convertEol, }; const args = { app, @@ -388,7 +404,7 @@ export const push: CommandDefinition< typeof options.env === 'string' ? [options.env] : options.env || [], - convertEol: options['convert-eol'] || false, + convertEol, }), ).catch(BuildError, e => { throw new ExpectedError(e.toString()); diff --git a/lib/utils/compose-types.d.ts b/lib/utils/compose-types.d.ts index 55568368..d2b3c9f5 100644 --- a/lib/utils/compose-types.d.ts +++ b/lib/utils/compose-types.d.ts @@ -47,6 +47,7 @@ export interface TaggedImage { } export interface ComposeOpts { + convertEol: boolean; dockerfilePath?: string; inlineLogs?: boolean; noParentCheck: boolean; diff --git a/lib/utils/compose.js b/lib/utils/compose.js index da19943d..5ba03b89 100644 --- a/lib/utils/compose.js +++ b/lib/utils/compose.js @@ -92,12 +92,24 @@ export function appendOptions(opts) { }, { signature: 'convert-eol', - description: `\ + description: isV12() + ? 'No-op and deprecated since balena CLI v12.0.0' + : `\ On Windows only, convert line endings from CRLF (Windows format) to LF (Unix format). \ Source files are not modified.`, boolean: true, alias: 'l', }, + ...(isV12() + ? [ + { + signature: 'noconvert-eol', + description: + "Don't convert line endings from CRLF (Windows format) to LF (Unix format).", + boolean: true, + }, + ] + : []), ]); } @@ -111,6 +123,7 @@ export function generateOpts(options) { projectName: options.projectName, projectPath, inlineLogs: !options.nologs && (!!options.logs || isV12()), + convertEol: isV12() ? !options['noconvert-eol'] : !!options['convert-eol'], dockerfilePath: options.dockerfile, nogitignore: !!options.nogitignore || isV12(), noParentCheck: options['noparent-check'], diff --git a/lib/utils/device/deploy.ts b/lib/utils/device/deploy.ts index 93fbea76..648e866c 100644 --- a/lib/utils/device/deploy.ts +++ b/lib/utils/device/deploy.ts @@ -178,6 +178,7 @@ export async function deployToDevice(opts: DeviceDeployOptions): Promise { globalLogger.logInfo(`Starting build on device ${opts.deviceHost}`); const project = await loadProject(globalLogger, { + convertEol: opts.convertEol, dockerfilePath: opts.dockerfilePath, noParentCheck: opts.noParentCheck, projectName: 'local', diff --git a/lib/utils/eol-conversion.ts b/lib/utils/eol-conversion.ts index b337df28..10291e6e 100644 --- a/lib/utils/eol-conversion.ts +++ b/lib/utils/eol-conversion.ts @@ -17,6 +17,7 @@ import { fs } from 'mz'; import Logger = require('./logger'); +import { isV12 } from './version'; const globalLogger = Logger.getLogger(); @@ -110,7 +111,9 @@ export async function readFileWithEolConversion( ); // And summary warning later globalLogger.deferredLog( - 'Windows-format line endings were detected in some files. Consider using the `--convert-eol` option.', + isV12() + ? 'Windows-format line endings were detected in some files, but were not converted due to `--noconvert-eol` option.' + : 'Windows-format line endings were detected in some files. Consider using the `--convert-eol` option.', Logger.Level.WARN, ); diff --git a/tests/commands/build.spec.ts b/tests/commands/build.spec.ts index a460782d..7eb2b3e0 100644 --- a/tests/commands/build.spec.ts +++ b/tests/commands/build.spec.ts @@ -84,9 +84,14 @@ describe('balena build', function() { it('should create the expected tar stream (single container)', async () => { const projectPath = path.join(projectsPath, 'no-docker-compose', 'basic'); + const isV12W = isWindows && isV12(); const expectedFiles: ExpectedTarStreamFiles = { 'src/start.sh': { fileSize: 89, type: 'file' }, - 'src/windows-crlf.sh': { fileSize: 70, type: 'file' }, + 'src/windows-crlf.sh': { + fileSize: isV12W ? 68 : 70, + testStream: isV12W ? expectStreamNoCRLF : undefined, + type: 'file', + }, Dockerfile: { fileSize: 88, type: 'file' }, 'Dockerfile-alt': { fileSize: 30, type: 'file' }, }; @@ -104,14 +109,17 @@ describe('balena build', function() { : '[Build] main Image size: 1.14 MB', ]; if (isWindows) { - expectedResponseLines.push( - `[Warn] CRLF (Windows) line endings detected in file: ${path.join( - projectPath, - 'src', - 'windows-crlf.sh', - )}`, - '[Warn] Windows-format line endings were detected in some files. Consider using the `--convert-eol` option.', - ); + const fname = path.join(projectPath, 'src', 'windows-crlf.sh'); + if (isV12()) { + expectedResponseLines.push( + `[Info] Converting line endings CRLF -> LF for file: ${fname}`, + ); + } else { + expectedResponseLines.push( + `[Warn] CRLF (Windows) line endings detected in file: ${fname}`, + '[Warn] Windows-format line endings were detected in some files. Consider using the `--convert-eol` option.', + ); + } } docker.expectGetInfo({}); await testDockerBuildStream({ @@ -129,6 +137,7 @@ describe('balena build', function() { it('should create the expected tar stream (--emulated)', async () => { const projectPath = path.join(projectsPath, 'no-docker-compose', 'basic'); + const isV12W = isWindows && isV12(); const transposedDockerfile = stripIndent` FROM busybox @@ -138,7 +147,11 @@ describe('balena build', function() { CMD ["/usr/src/start.sh"]` + '\n'; const expectedFiles: ExpectedTarStreamFiles = { 'src/start.sh': { fileSize: 89, type: 'file' }, - 'src/windows-crlf.sh': { fileSize: 70, type: 'file' }, + 'src/windows-crlf.sh': { + fileSize: isV12W ? 68 : 70, + testStream: isV12W ? expectStreamNoCRLF : undefined, + type: 'file', + }, Dockerfile: { fileSize: transposedDockerfile.length, type: 'file', @@ -162,14 +175,17 @@ describe('balena build', function() { '[Success] Build succeeded!', ]; if (isWindows) { - expectedResponseLines.push( - `[Warn] CRLF (Windows) line endings detected in file: ${path.join( - projectPath, - 'src', - 'windows-crlf.sh', - )}`, - '[Warn] Windows-format line endings were detected in some files. Consider using the `--convert-eol` option.', - ); + const fname = path.join(projectPath, 'src', 'windows-crlf.sh'); + if (isV12()) { + expectedResponseLines.push( + `[Info] Converting line endings CRLF -> LF for file: ${fname}`, + ); + } else { + expectedResponseLines.push( + `[Warn] CRLF (Windows) line endings detected in file: ${fname}`, + '[Warn] Windows-format line endings were detected in some files. Consider using the `--convert-eol` option.', + ); + } } const arch = 'rpi'; const deviceType = 'raspberry-pi'; @@ -207,13 +223,14 @@ describe('balena build', function() { } }); - it('should create the expected tar stream (single container, --convert-eol)', async () => { + it('should create the expected tar stream (single container, --[no]convert-eol)', async () => { const projectPath = path.join(projectsPath, 'no-docker-compose', 'basic'); + const eol = isWindows && !isV12(); const expectedFiles: ExpectedTarStreamFiles = { 'src/start.sh': { fileSize: 89, type: 'file' }, 'src/windows-crlf.sh': { - fileSize: isWindows ? 68 : 70, - testStream: isWindows ? expectStreamNoCRLF : undefined, + fileSize: eol ? 68 : 70, + testStream: eol ? expectStreamNoCRLF : undefined, type: 'file', }, Dockerfile: { fileSize: 88, type: 'file' }, @@ -233,17 +250,23 @@ describe('balena build', function() { : '[Build] main Image size: 1.14 MB', ]; if (isWindows) { - expectedResponseLines.push( - `[Info] Converting line endings CRLF -> LF for file: ${path.join( - projectPath, - 'src', - 'windows-crlf.sh', - )}`, - ); + const fname = path.join(projectPath, 'src', 'windows-crlf.sh'); + if (isV12()) { + expectedResponseLines.push( + `[Warn] CRLF (Windows) line endings detected in file: ${fname}`, + '[Warn] Windows-format line endings were detected in some files, but were not converted due to `--noconvert-eol` option.', + ); + } else { + expectedResponseLines.push( + `[Info] Converting line endings CRLF -> LF for file: ${fname}`, + ); + } } docker.expectGetInfo({}); await testDockerBuildStream({ - commandLine: `build ${projectPath} --deviceType nuc --arch amd64 --convert-eol`, + commandLine: isV12() + ? `build ${projectPath} --deviceType nuc --arch amd64 --noconvert-eol` + : `build ${projectPath} --deviceType nuc --arch amd64 --convert-eol`, dockerMock: docker, expectedFilesByService: { main: expectedFiles }, expectedQueryParamsByService: { main: commonQueryParams }, diff --git a/tests/commands/deploy.spec.ts b/tests/commands/deploy.spec.ts index e2fadbec..be96d28a 100644 --- a/tests/commands/deploy.spec.ts +++ b/tests/commands/deploy.spec.ts @@ -25,7 +25,7 @@ import * as sinon from 'sinon'; import { isV12 } from '../../build/utils/version'; import { BalenaAPIMock } from '../balena-api-mock'; -import { testDockerBuildStream } from '../docker-build'; +import { expectStreamNoCRLF, testDockerBuildStream } from '../docker-build'; import { DockerMock, dockerResponsePath } from '../docker-mock'; import { cleanOutput, runCommand, switchSentry } from '../helpers'; import { ExpectedTarStreamFiles } from '../projects'; @@ -106,9 +106,14 @@ describe('balena deploy', function() { it('should create the expected --build tar stream (single container)', async () => { const projectPath = path.join(projectsPath, 'no-docker-compose', 'basic'); + const isV12W = isWindows && isV12(); const expectedFiles: ExpectedTarStreamFiles = { 'src/start.sh': { fileSize: 89, type: 'file' }, - 'src/windows-crlf.sh': { fileSize: 70, type: 'file' }, + 'src/windows-crlf.sh': { + fileSize: isV12W ? 68 : 70, + testStream: isV12W ? expectStreamNoCRLF : undefined, + type: 'file', + }, Dockerfile: { fileSize: 88, type: 'file' }, 'Dockerfile-alt': { fileSize: 30, type: 'file' }, }; @@ -123,14 +128,17 @@ describe('balena deploy', function() { `[Info] Creating default composition with source: "${projectPath}"`, ]; if (isWindows) { - expectedResponseLines.push( - `[Warn] CRLF (Windows) line endings detected in file: ${path.join( - projectPath, - 'src', - 'windows-crlf.sh', - )}`, - '[Warn] Windows-format line endings were detected in some files. Consider using the `--convert-eol` option.', - ); + const fname = path.join(projectPath, 'src', 'windows-crlf.sh'); + if (isV12()) { + expectedResponseLines.push( + `[Info] Converting line endings CRLF -> LF for file: ${fname}`, + ); + } else { + expectedResponseLines.push( + `[Warn] CRLF (Windows) line endings detected in file: ${fname}`, + '[Warn] Windows-format line endings were detected in some files. Consider using the `--convert-eol` option.', + ); + } } api.expectPatchImage({}); @@ -189,7 +197,7 @@ describe('balena deploy', function() { }); await testDockerBuildStream({ - commandLine: `deploy testApp --build --source ${projectPath} -G`, + commandLine: `deploy testApp --build --source ${projectPath} --noconvert-eol -G`, dockerMock: docker, expectedFilesByService: { main: expectedFiles }, expectedQueryParamsByService: { main: commonQueryParams }, diff --git a/tests/commands/push.spec.ts b/tests/commands/push.spec.ts index f7bf8d93..8c3a7aec 100644 --- a/tests/commands/push.spec.ts +++ b/tests/commands/push.spec.ts @@ -109,9 +109,14 @@ describe('balena push', function() { it('should create the expected tar stream (single container)', async () => { const projectPath = path.join(projectsPath, 'no-docker-compose', 'basic'); + const isV12W = isWindows && isV12(); const expectedFiles: ExpectedTarStreamFiles = { 'src/start.sh': { fileSize: 89, type: 'file' }, - 'src/windows-crlf.sh': { fileSize: 70, type: 'file' }, + 'src/windows-crlf.sh': { + fileSize: isV12W ? 68 : 70, + testStream: isV12W ? expectStreamNoCRLF : undefined, + type: 'file', + }, Dockerfile: { fileSize: 88, type: 'file' }, 'Dockerfile-alt': { fileSize: 30, type: 'file' }, }; @@ -123,14 +128,17 @@ describe('balena push', function() { ); const expectedResponseLines = [...commonResponseLines[responseFilename]]; if (isWindows) { - expectedResponseLines.push( - `[Warn] CRLF (Windows) line endings detected in file: ${path.join( - projectPath, - 'src', - 'windows-crlf.sh', - )}`, - '[Warn] Windows-format line endings were detected in some files. Consider using the `--convert-eol` option.', - ); + const fname = path.join(projectPath, 'src', 'windows-crlf.sh'); + if (isV12()) { + expectedResponseLines.push( + `[Info] Converting line endings CRLF -> LF for file: ${fname}`, + ); + } else { + expectedResponseLines.push( + `[Warn] CRLF (Windows) line endings detected in file: ${fname}`, + '[Warn] Windows-format line endings were detected in some files. Consider using the `--convert-eol` option.', + ); + } } await testPushBuildStream({ @@ -165,7 +173,7 @@ describe('balena push', function() { await testPushBuildStream({ builderMock: builder, - commandLine: `push testApp -s ${projectPath} -R ${regSecretsPath} --dockerfile Dockerfile-alt --nogitignore`, + commandLine: `push testApp -s ${projectPath} -R ${regSecretsPath} --dockerfile Dockerfile-alt --noconvert-eol --nogitignore`, expectedFiles, expectedQueryParams, expectedResponseLines: commonResponseLines[responseFilename], @@ -175,14 +183,15 @@ describe('balena push', function() { }); }); - it('should create the expected tar stream (single container, --convert-eol)', async () => { + it('should create the expected tar stream (single container, --[no]convert-eol)', async () => { const projectPath = path.join(projectsPath, 'no-docker-compose', 'basic'); + const eol = isWindows && !isV12(); const expectedFiles: ExpectedTarStreamFiles = { 'src/start.sh': { fileSize: 89, type: 'file' }, 'src/windows-crlf.sh': { - fileSize: isWindows ? 68 : 70, + fileSize: eol ? 68 : 70, + testStream: eol ? expectStreamNoCRLF : undefined, type: 'file', - testStream: isWindows ? expectStreamNoCRLF : undefined, }, Dockerfile: { fileSize: 88, type: 'file' }, 'Dockerfile-alt': { fileSize: 30, type: 'file' }, @@ -195,18 +204,24 @@ describe('balena push', function() { ); const expectedResponseLines = [...commonResponseLines[responseFilename]]; if (isWindows) { - expectedResponseLines.push( - `[Info] Converting line endings CRLF -> LF for file: ${path.join( - projectPath, - 'src', - 'windows-crlf.sh', - )}`, - ); + const fname = path.join(projectPath, 'src', 'windows-crlf.sh'); + if (isV12()) { + expectedResponseLines.push( + `[Warn] CRLF (Windows) line endings detected in file: ${fname}`, + '[Warn] Windows-format line endings were detected in some files, but were not converted due to `--noconvert-eol` option.', + ); + } else { + expectedResponseLines.push( + `[Info] Converting line endings CRLF -> LF for file: ${fname}`, + ); + } } await testPushBuildStream({ builderMock: builder, - commandLine: `push testApp -s ${projectPath} -R ${regSecretsPath} -l`, + commandLine: isV12() + ? `push testApp -s ${projectPath} -R ${regSecretsPath} --noconvert-eol` + : `push testApp -s ${projectPath} -R ${regSecretsPath} -l`, expectedFiles, expectedQueryParams: commonQueryParams, expectedResponseLines, @@ -322,6 +337,10 @@ describe('balena push', function() { }); }); + // NOTE: if this test or other tests involving symbolic links fail on Windows + // (with a mismatched fileSize 13 vs 5 for 'symlink-a.txt'), ensure that the + // `core.symlinks` property is set to `true` in the `.git/config` file. Ref: + // https://git-scm.com/docs/git-config#Documentation/git-config.txt-coresymlinks it('should create the expected tar stream (single container, symbolic links, --nogitignore)', async () => { const projectPath = path.join( projectsPath, diff --git a/tests/utils/tarDirectory.spec.ts b/tests/utils/tarDirectory.spec.ts index 410caf9b..53bf77af 100644 --- a/tests/utils/tarDirectory.spec.ts +++ b/tests/utils/tarDirectory.spec.ts @@ -57,6 +57,10 @@ describe('compare new and old tarDirectory implementations', function() { await setupDockerignoreTestData({ cleanup: true }); }); + // NOTE: if this test or other tests involving symbolic links fail on Windows + // (with a mismatched fileSize 13 vs 5 for 'symlink-a.txt'), ensure that the + // `core.symlinks` property is set to `true` in the `.git/config` file. Ref: + // https://git-scm.com/docs/git-config#Documentation/git-config.txt-coresymlinks it('should produce the expected file list', async function() { const dockerignoreProjDir = path.join( projectsPath,