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

Copy instructions should copy by default #702

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
name: Release

on:
# Temporarily disabling scheduled releases to evaluate whether any other major changes are desired.
# release daily at 8am UTC (midnight or 1am pacific)
# https://crontab-generator.org/
schedule:
- cron: '0 8 * * *'
# schedule:
# - cron: '0 8 * * *'
# or manual trigger
workflow_dispatch:

Expand Down
11 changes: 11 additions & 0 deletions change/change-e0d6790d-3823-4990-8bff-44914b63d4d3.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"type": "major",
"comment": "Revert to original copy instructions behavior of copying by default. To create symlinks instead, pass `symlink: true`.",
"packageName": "just-scripts",
"email": "[email protected]",
"dependentChangeType": "patch"
}
]
}
7 changes: 0 additions & 7 deletions packages/just-scripts/src/arrayUtils/uniqueValues.ts

This file was deleted.

23 changes: 11 additions & 12 deletions packages/just-scripts/src/copy/CopyInstruction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@ export interface CopyInstruction {
destinationFilePath: string;

/**
* Set to true if a copy or merge should be performed, false if a symlink should be created.
* If multiple source files are specified (i.e. a merge), this must be true or undefined.
* The default value of undefined is equivalent to true for a merge, false in all other cases.
* Set to true to create symlinks rather than copying.
* If multiple source files are specified (i.e. a merge), this must be false or unset.
*/
noSymlink?: boolean;
symlink?: boolean;
}

export interface CopyConfig {
Expand All @@ -34,12 +33,12 @@ export interface CopyConfig {
export function copyFilesToDestinationDirectory(
sourceFilePaths: string | string[],
destinationDirectory: string,
noSymlinks?: boolean,
symlink?: boolean,
): CopyInstruction[] {
return arrayify(sourceFilePaths).map(sourceName => ({
sourceFilePath: normalize(sourceName),
destinationFilePath: join(destinationDirectory, basename(sourceName)),
noSymlink: noSymlinks,
symlink,
}));
}

Expand All @@ -52,9 +51,9 @@ export function copyFileToDestinationDirectoryWithRename(
sourceFilePath: string,
destinationName: string,
destinationDirectory: string,
noSymlink?: boolean,
symlink?: boolean,
): CopyInstruction[] {
return [{ sourceFilePath, destinationFilePath: join(destinationDirectory, destinationName), noSymlink }];
return [{ sourceFilePath, destinationFilePath: join(destinationDirectory, destinationName), symlink }];
}

/**
Expand All @@ -65,12 +64,12 @@ export function copyFileToDestinationDirectoryWithRename(
export function copyFilesToDestinationDirectoryWithRename(
instrs: { sourceFilePath: string; destinationName: string }[],
destinationDirectory: string,
noSymlinks?: boolean,
symlink?: boolean,
): CopyInstruction[] {
return instrs.map(instr => ({
sourceFilePath: instr.sourceFilePath,
destinationFilePath: join(destinationDirectory, instr.destinationName),
noSymlink: noSymlinks,
symlink,
}));
}

Expand All @@ -82,7 +81,7 @@ export function copyFilesInDirectory(
sourceDirectoryPath: string,
outputDirectoryPath: string,
filterFunction?: (file: string) => boolean,
noSymlinks?: boolean,
symlink?: boolean,
): CopyInstruction[] {
let files = readdirSync(sourceDirectoryPath);

Expand All @@ -92,7 +91,7 @@ export function copyFilesInDirectory(
return files.map(file => ({
sourceFilePath: join(sourceDirectoryPath, file),
destinationFilePath: join(outputDirectoryPath, file),
noSymlink: noSymlinks,
symlink,
}));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as mockfs from 'mock-fs';
import * as path from 'path';
import * as fs from 'fs';
// import { dirSync, fileSync, DirResult, FileResult } from 'tmp';
import { executeCopyInstructions } from '../executeCopyInstructions';
import { CopyInstruction } from '../CopyInstruction';

Expand Down Expand Up @@ -35,6 +34,7 @@ describe('executeCopyInstructions functional tests', () => {
const copyInstruction: CopyInstruction = {
sourceFilePath: sourceFilePath1,
destinationFilePath: destFilePath,
symlink: true,
};

expect(fs.existsSync(destFilePath)).toBeFalsy();
Expand All @@ -52,7 +52,6 @@ describe('executeCopyInstructions functional tests', () => {
const copyInstruction: CopyInstruction = {
sourceFilePath: [sourceFilePath1],
destinationFilePath: destFilePath,
noSymlink: true,
};

expect(fs.existsSync(destFilePath)).toBeFalsy();
Expand Down Expand Up @@ -89,7 +88,7 @@ describe('executeCopyInstructions functional tests', () => {
const copyInstruction: CopyInstruction = {
sourceFilePath: [sourceFilePath1, sourceFilePath2],
destinationFilePath: destFilePath,
noSymlink: false,
symlink: true,
};

const promise = executeCopyInstructions({
Expand Down
22 changes: 8 additions & 14 deletions packages/just-scripts/src/copy/executeCopyInstructions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { dirname, resolve } from 'path';
import { readFile, writeFile, copy, ensureDir, ensureSymlink } from 'fs-extra';
import { CopyInstruction, CopyConfig } from './CopyInstruction';
import { arrayify } from '../arrayUtils/arrayify';
import { uniqueValues } from '../arrayUtils/uniqueValues';

/**
* Function containing the core code for the copy task with a given config.
Expand All @@ -17,34 +16,29 @@ export async function executeCopyInstructions(config: CopyConfig | undefined): P

function validateConfig(copyInstructions: CopyInstruction[]) {
copyInstructions.forEach(instr => {
if (instr.noSymlink === false && Array.isArray(instr.sourceFilePath) && instr.sourceFilePath.length > 1) {
if (instr.symlink && Array.isArray(instr.sourceFilePath) && instr.sourceFilePath.length > 1) {
throw new Error('Multiple source files cannot be specified when making a symlink');
}
});
}

function createDirectories(copyInstructions: CopyInstruction[]) {
return Promise.all(
uniqueValues(copyInstructions.map(instruction => dirname(instruction.destinationFilePath))).map(dirname =>
ensureDir(dirname),
),
);
const directories = new Set(copyInstructions.map(instruction => dirname(instruction.destinationFilePath)));
return Promise.all([...directories].map(dirname => ensureDir(dirname)));
}

function executeSingleCopyInstruction(copyInstruction: CopyInstruction) {
async function executeSingleCopyInstruction(copyInstruction: CopyInstruction) {
const sourceFileNames = arrayify(copyInstruction.sourceFilePath);

// source and dest are 1-to-1? perform binary copy or symlink as desired.
if (sourceFileNames.length === 1) {
if (copyInstruction.noSymlink) {
return copy(sourceFileNames[0], copyInstruction.destinationFilePath);
} else {
if (copyInstruction.symlink) {
return ensureSymlink(resolve(sourceFileNames[0]), copyInstruction.destinationFilePath);
}
return copy(sourceFileNames[0], copyInstruction.destinationFilePath);
}

// perform text merge operation.
return Promise.all(sourceFileNames.map(fileName => readFile(fileName))).then(fileContents => {
return writeFile(copyInstruction.destinationFilePath, fileContents.join('\n'));
});
const sourceFiles = await Promise.all(sourceFileNames.map(fileName => readFile(fileName)));
return writeFile(copyInstruction.destinationFilePath, sourceFiles.join('\n'));
}