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

Fix cache #354

Merged
merged 4 commits into from
Oct 27, 2023
Merged

Fix cache #354

merged 4 commits into from
Oct 27, 2023

Conversation

varunsh-coder
Copy link
Member

No description provided.

@varunsh-coder varunsh-coder merged commit b2d3a9f into rc-v2.6.1 Oct 27, 2023
2 checks passed
Copy link
Contributor

@step-security-bot step-security-bot left a comment

Choose a reason for hiding this comment

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

Please find StepSecurity AI-CodeWise code comments inline or below.

dist/pre/index.js

Please refer to 7 inline comments.

src/setup.ts

Please refer to 1 inline comments.

Feedback

We appreciate your feedback in helping us improve the service! To provide feedback, please use emojis on this comment. If you find a comment helpful, give it a 👍. If they aren't useful, kindly express that with a 👎. If you have questions or detailed feedback, please create n GitHub issue in StepSecurity/AI-CodeWise.

throw error;
}
else if (typedError.name === ReserveCacheError.name) {
core.info(`Failed to save: ${typedError.message}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

[Low]Update the log statement to a warning

The 'Failed to save:' log statement in the saveCache function logs as information with a severity level, yet it conveys sufficient information that may be important for debugging issues and must be logged as a warning to get developers' attention. Convert the log statement to a warning with a severity level and proper contextual message

}
}
exports.ReserveCacheError = ReserveCacheError;
function checkPaths(paths) {
Copy link
Contributor

Choose a reason for hiding this comment

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

[Medium]Consider including this refactored function into your pull request

.


throw new ValidationError(`Path Validation Error: At least one directory or file path is required`);
}
}
function checkKey(key) {
Copy link
Contributor

Choose a reason for hiding this comment

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

[Medium]Consider including this refactored function into your pull request

.


*
* @returns boolean return true if Actions cache service feature is available, otherwise false
*/
function isFeatureAvailable() {
Copy link
Contributor

Choose a reason for hiding this comment

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

[Medium]Consider including this refactored function into your pull request

.

async function isFeatureAvailable() {
    return !!process.env['ACTIONS_CACHE_URL'];
}
exports.isFeatureAvailable = isFeatureAvailable;

const constants_1 = __nccwpck_require__(8840);
const IS_WINDOWS = process.platform === 'win32';
// Returns tar path and type: BSD or GNU
function getTarPath() {
Copy link
Contributor

Choose a reason for hiding this comment

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

[Medium]Consider including this refactored function into your pull request

.

async function getTarPath() {
    switch (process.platform) {
        case 'win32': {
            const gnuTar = await utils.getGnuTarPathOnWindows();
            const systemTar = constants_1.SystemTarPathOnWindows;
            if (gnuTar) {
                // Use GNUtar as default on windows
                return { path: gnuTar, type: constants_1.ArchiveToolType.GNU };
            }
            else if (fs_1.existsSync(systemTar)) {
                return { path: systemTar, type: constants_1.ArchiveToolType.BSD };
            }
            break;
        }
        case 'darwin': {
            const gnuTar = await io.which('gtar', false);
            if (gnuTar) {
                // fix permission denied errors when extracting BSD tar archive with GNU tar - https://github.com/actions/cache/issues/527
                return { path: gnuTar, type: constants_1.ArchiveToolType.GNU };
            }
            else {
                return {
                    path: await io.which('tar', true),
                    type: constants_1.ArchiveToolType.BSD
                };
            }
        }
        default:
            break;
    }
    // Default assumption is GNU tar is present in path
    return {
        path: await io.which('tar', true),
        type: constants_1.ArchiveToolType.GNU
    };
}

});
}
// Return arguments for tar as per tarPath, compressionMethod, method type and os
function getTarArgs(tarPath, compressionMethod, type, archivePath = '') {
Copy link
Contributor

Choose a reason for hiding this comment

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

[Medium]Consider including this refactored function into your pull request

.

async function getTarArgs(tarPath, compressionMethod, type, archivePath = '') {
    const args = [`"${tarPath.path}"`];
    const cacheFileName = utils.getCacheFileName(compressionMethod);
    const tarFile = 'cache.tar';
    const workingDirectory = getWorkingDirectory();
    // Speficic args for BSD tar on windows for workaround
    const BSD_TAR_ZSTD = tarPath.type === constants_1.ArchiveToolType.BSD &&
        compressionMethod !== constants_1.CompressionMethod.Gzip &&
        IS_WINDOWS;
    // Method specific args
    switch(type) {
        case 'create':
            args.push('--posix', '-cf', BSD_TAR_ZSTD ? tarFile : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '--exclude', BSD_TAR_ZSTD ? tarFile : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', '-C', workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '--files-from', constants_1.ManifestFilename);
            break;
        case 'extract':
            args.push('-xf', BSD_TAR_ZSTD ? tarFile : archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', '-C', workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/'));
            break;
        case 'list':
            args.push('-tf', BSD_TAR_ZSTD ? tarFile : archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P');
            break;
    }
    // Platform specific args
    if (tarPath.type === constants_1.ArchiveToolType.GNU) {
        switch(process.platform) {
            case 'win32':
                args.push('--force-local');
                break;
            case 'darwin':
                args.push('--delay-directory-restore');
                break;
        }
    }
    return args;
}

});
}
// Returns commands to run tar and compression program
function getCommands(compressionMethod, type, archivePath = '') {
Copy link
Contributor

Choose a reason for hiding this comment

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

[Medium]Consider including this refactored function into your pull request

.

async function getCommands(compressionMethod, type, archivePath = '') {
    let args;
    const tarPath = await getTarPath();
    const tarArgs = await getTarArgs(tarPath, compressionMethod, type, archivePath);
    const compressionArgs = type !== 'create' ? await getDecompressionProgram(tarPath, compressionMethod, archivePath) : await getCompressionProgram(tarPath, compressionMethod);
    const BSD_TAR_ZSTD = tarPath.type === constants_1.ArchiveToolType.BSD &&
        compressionMethod !== constants_1.CompressionMethod.Gzip &&
        IS_WINDOWS;
    if (BSD_TAR_ZSTD && type !== 'create') {
        args = [[...compressionArgs].join(' '), [...tarArgs].join(' ')]
    } else {
        args = [[...tarArgs].join(' '), [...compressionArgs].join(' ')]
    }
    if (BSD_TAR_ZSTD) {
        return args
    }
    return [args.join(' ')]
}

@@ -19,7 +19,7 @@ import {
} from "./cache";
import { Configuration, PolicyResponse } from "./interfaces";
import { fetchPolicy, mergeConfigs } from "./policy-utils";

import * as cache from "@actions/cache";
Copy link
Contributor

Choose a reason for hiding this comment

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

[Medium]Consider including this refactored function into your pull request

.

import * as cache from "@actions/cache";

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.

None yet

2 participants