From f6a7e66dd04d941fbb0bb5c1704414ddf211953f Mon Sep 17 00:00:00 2001
From: Marcus Crane <marcus.crane@lightspeedhq.com>
Date: Sat, 20 Aug 2022 23:24:32 +1200
Subject: [PATCH] Incorporate requested changes

Signed-off-by: Marcus Crane <marcus.crane@lightspeedhq.com>
---
 .../lib/files/serializeDirectoryContents.ts   | 32 +++++++++----------
 .../builtin/publish/githubPullRequest.ts      | 28 +++++++++++-----
 2 files changed, 35 insertions(+), 25 deletions(-)

diff --git a/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts b/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts
index b3d565973e..46c1002259 100644
--- a/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts
+++ b/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts
@@ -17,7 +17,7 @@
 import fs from 'fs-extra';
 import globby from 'globby';
 import limiterFactory from 'p-limit';
-import { join as joinPath } from 'path';
+import { resolveSafeChildPath } from '@backstage/backend-common';
 import { SerializedFile } from './types';
 
 const DEFAULT_GLOB_PATTERNS = ['./**', '!.git'];
@@ -57,23 +57,21 @@ export async function serializeDirectoryContents(
     paths
       .filter(({ dirent }) => !dirent.isDirectory())
       .filter(({ dirent, path }) => {
-        if (!dirent.isSymbolicLink()) return true
-        if (!fs.existsSync(joinPath(sourcePath, path))) return true  // We only want symlinks that DO NOT exist
-        return false
+        if (!dirent.isSymbolicLink()) return true;
+        if (!fs.existsSync(resolveSafeChildPath(sourcePath, path))) return true; // We only want symlinks that DO NOT exist (yet)
+        return false;
       })
       .map(async ({ dirent, path, stats }) => ({
-      path,
-      content: await limiter(async () => {
-        const absFilePath = joinPath(sourcePath, path)
-        // Treat readlink as an explicit Buffer instead of implict utf-8 for consistency between types
-        const readLinkConf = { options: { encoding: null }}
-        if (dirent.isSymbolicLink()) {
-          return fs.readlink(absFilePath, readLinkConf)
-        }
-        return fs.readFile(absFilePath)
-      }),
-      executable: isExecutable(stats?.mode),
-      symlink: dirent.isSymbolicLink(),
-    })),
+        path,
+        content: await limiter(async () => {
+          const absFilePath = resolveSafeChildPath(sourcePath, path);
+          if (dirent.isSymbolicLink()) {
+            return fs.readlinkSync(absFilePath, 'buffer');
+          }
+          return fs.readFile(absFilePath);
+        }),
+        executable: isExecutable(stats?.mode),
+        symlink: dirent.isSymbolicLink(),
+      })),
   );
 }
diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts
index 8dc4fd1c8c..b2c7a60120 100644
--- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts
+++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts
@@ -26,7 +26,10 @@ import { InputError, CustomErrorBase } from '@backstage/errors';
 import { createPullRequest } from 'octokit-plugin-create-pull-request';
 import { resolveSafeChildPath } from '@backstage/backend-common';
 import { getOctokitOptions } from '../github/helpers';
-import { serializeDirectoryContents } from '../../../../lib/files';
+import {
+  SerializedFile,
+  serializeDirectoryContents,
+} from '../../../../lib/files';
 import { Logger } from 'winston';
 
 export type Encoding = 'utf-8' | 'base64';
@@ -249,23 +252,32 @@ export const createPublishGithubPullRequestAction = ({
       const directoryContents = await serializeDirectoryContents(fileRoot, {
         gitignore: true,
       });
+
+      const determineFileMode = (file: SerializedFile): string => {
+        if (file.symlink) return '120000';
+        if (file.executable) return '100755';
+        return '100644';
+      };
+
+      const determineFileEncoding = (file: SerializedFile): string =>
+        file.symlink ? 'utf-8' : 'base64';
+
       const files = Object.fromEntries(
         directoryContents.map(file => [
           targetPath ? path.posix.join(targetPath, file.path) : file.path,
           {
             // See the properties of tree items
             // in https://docs.github.com/en/rest/reference/git#trees
-            mode: file.symlink ? '120000' : (file.executable ? '100755' : '100644'),
-            // Always use base64 encoding to avoid doubling a binary file in size
+            mode: determineFileMode(file),
+            // Always use base64 encoding where possible to avoid doubling a binary file in size
             // due to interpreting a binary file as utf-8 and sending github
-            // the utf-8 encoded content.
+            // the utf-8 encoded content. Symlinks are kept as utf-8 to avoid them
+            // being formatted as a series of scrambled characters
             //
             // For example, the original gradle-wrapper.jar is 57.8k in https://github.com/kennethzfeng/pull-request-test/pull/5/files.
             // Its size could be doubled to 98.3K (See https://github.com/kennethzfeng/pull-request-test/pull/4/files)
-            encoding: file.symlink ? 'utf-8' : 'base64',
-            content: file.content.toString(
-              (file.symlink ? 'utf-8' : 'base64')
-            ),
+            encoding: determineFileEncoding(file),
+            content: file.content.toString(determineFileEncoding(file)),
           },
         ]),
       );