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.

Sign up for GitHub

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

only copy theme resources when they actually change #990

Merged
merged 3 commits into from
Sep 8, 2018
Merged

only copy theme resources when they actually change #990

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a42701e
only copy theme resources when they actually change
bridiver Sep 6, 2018
5843ef3
no_branding_update flag is no longer relevant
bridiver Sep 6, 2018
43bb783
use checksum for channel dependent files
bridiver Sep 7, 2018
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
31 changes: 3 additions & 28 deletions lib/build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,8 @@ const touchOverriddenFiles = () => {
return true
}

const walkSync = (dir, filelist = []) => {
fs.readdirSync(dir).forEach(file => {
if (fs.statSync(path.join(dir, file)).isDirectory()) {
filelist = walkSync(path.join(dir, file), filelist)
} else if (applyFileFilter(file)) {
filelist = filelist.concat(path.join(dir, file))
}
})
return filelist
}

const chromiumSrcDir = path.join(config.srcDir, 'brave', 'chromium_src')
var sourceFiles = walkSync(chromiumSrcDir)
var sourceFiles = util.walkSync(chromiumSrcDir, applyFileFilter)

// Touch original files by updating mtime.
const chromiumSrcDirLen = chromiumSrcDir.length
Expand Down Expand Up @@ -63,19 +52,8 @@ const touchOverriddenVectorIconFiles = () => {
return true
}

const walkSync = (dir, filelist = []) => {
fs.readdirSync(dir).forEach(file => {
if (fs.statSync(path.join(dir, file)).isDirectory()) {
filelist = walkSync(path.join(dir, file), filelist)
} else if (applyFileFilter(file)) {
filelist = filelist.concat(path.join(dir, file))
}
})
return filelist
}

const braveVectorIconsDir = path.join(config.srcDir, 'brave', 'vector_icons')
var braveVectorIconFiles = walkSync(braveVectorIconsDir)
var braveVectorIconFiles = util.walkSync(braveVectorIconsDir, applyFileFilter)

// Touch original files by updating mtime.
const braveVectorIconsDirLen = braveVectorIconsDir.length
Expand Down Expand Up @@ -116,10 +94,7 @@ const build = (buildConfig = config.defaultBuildConfig, options) => {

touchOverriddenFiles()
touchOverriddenVectorIconFiles()

if (!options.no_branding_update) {
util.updateBranding()
}
util.updateBranding()

util.buildTarget()
}
Expand Down
5 changes: 1 addition & 4 deletions lib/createDist.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@ const createDist = (buildConfig = config.defaultBuildConfig, options) => {
config.buildConfig = buildConfig
config.update(options)

if (!options.no_branding_update) {
util.updateBranding()
}

util.updateBranding()
fs.removeSync(path.join(config.outputDir, 'dist'))
config.buildTarget = 'create_dist'
util.buildTarget()
Expand Down
89 changes: 66 additions & 23 deletions lib/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,47 +66,79 @@ const util = {
const braveBrowserDir = path.join(config.projects['brave-core'].dir, 'browser')
const braveAppVectorIconsDir = path.join(config.projects['brave-core'].dir, 'vector_icons', 'chrome', 'app')

// The following 3 entries we map to the same name, not the chromium equivalent name for copying back
autoGeneratedBraveToChromiumMapping[path.join(braveAppDir, 'brave_strings.grd')] = path.join(chromeAppDir, 'brave_strings.grd')
autoGeneratedBraveToChromiumMapping[path.join(braveAppDir, 'settings_brave_strings.grdp')] = path.join(chromeAppDir, 'settings_brave_strings.grdp')
autoGeneratedBraveToChromiumMapping[path.join(braveAppDir, 'components_brave_strings.grd')] = path.join(config.srcDir, 'components', 'components_brave_strings.grd')
let fileMap = {}
fileMap[path.join(braveAppDir, 'brave_strings.grd')] = path.join(chromeAppDir, 'brave_strings.grd')
fileMap[path.join(braveAppDir, 'settings_brave_strings.grdp')] = path.join(chromeAppDir, 'settings_brave_strings.grdp')
fileMap[path.join(braveAppDir, 'components_brave_strings.grd')] = path.join(config.srcDir, 'components', 'components_brave_strings.grd')

// Copy each grd back to Chromium src dir
Object.entries(autoGeneratedBraveToChromiumMapping).forEach(([bravePath, chromiumPath]) =>
fs.copySync(bravePath, chromiumPath))

// Copy xtb files for:
// brave/app/resources/chromium_strings*.xtb
// brave/app/strings/components_chromium_strings*.xtb
// brave/app/resources/generated_resoruces*.xtb
fs.copySync(path.join(braveAppDir, 'resources'), path.join(chromeAppDir, 'resources'))
fs.copySync(path.join(braveAppDir, 'strings'), path.join(chromeComponentsDir, 'strings'))
fileMap[path.join(braveAppDir, 'resources')] = path.join(chromeAppDir, 'resources')
fileMap[path.join(braveAppDir, 'strings')] = path.join(chromeComponentsDir, 'strings')

fs.copySync(path.join(braveAppDir, 'theme', 'brave'), path.join(chromeAppDir, 'theme', 'brave'))
fs.copySync(path.join(braveAppDir, 'theme', 'default_100_percent', 'brave'), path.join(chromeAppDir, 'theme', 'default_100_percent', 'brave'))
fs.copySync(path.join(braveAppDir, 'theme', 'default_200_percent', 'brave'), path.join(chromeAppDir, 'theme', 'default_200_percent', 'brave'))
fileMap[path.join(braveAppDir, 'theme', 'brave')] = path.join(chromeAppDir, 'theme', 'brave')
fileMap[path.join(braveAppDir, 'theme', 'default_100_percent', 'brave')] = path.join(chromeAppDir, 'theme', 'default_100_percent', 'brave')
fileMap[path.join(braveAppDir, 'theme', 'default_200_percent', 'brave')] = path.join(chromeAppDir, 'theme', 'default_200_percent', 'brave')
// By overwriting, we don't need to modify some grd files.
fs.copySync(path.join(braveAppDir, 'theme', 'brave'), path.join(chromeAppDir, 'theme', 'chromium'))
fs.copySync(path.join(braveAppDir, 'theme', 'default_100_percent', 'brave'), path.join(chromeAppDir, 'theme', 'default_100_percent', 'chromium'))
fs.copySync(path.join(braveAppDir, 'theme', 'default_200_percent', 'brave'), path.join(chromeAppDir, 'theme', 'default_200_percent', 'chromium'))
fs.copySync(path.join(braveComponentsDir, 'resources', 'default_100_percent', 'brave'), path.join(chromeComponentsDir, 'resources', 'default_100_percent', 'chromium'))
fs.copySync(path.join(braveComponentsDir, 'resources', 'default_200_percent', 'brave'), path.join(chromeComponentsDir, 'resources', 'default_200_percent', 'chromium'))
fs.copySync(path.join(braveAppVectorIconsDir, 'vector_icons', 'brave'), path.join(chromeAppDir, 'vector_icons', 'brave'))
fs.copySync(path.join(braveResourcesDir, 'settings', 'brave_page_visibility.js'), path.join(chromeResourcesDir, 'settings', 'brave_page_visibility.js'))
fs.copySync(path.join(braveResourcesDir, 'settings', 'brave_appearance_page'), path.join(chromeResourcesDir, 'settings', 'brave_appearance_page'))
fileMap[path.join(braveAppDir, 'theme', 'brave')] = path.join(chromeAppDir, 'theme', 'chromium')
fileMap[path.join(braveAppDir, 'theme', 'default_100_percent', 'brave')] = path.join(chromeAppDir, 'theme', 'default_100_percent', 'chromium')
fileMap[path.join(braveAppDir, 'theme', 'default_200_percent', 'brave')] = path.join(chromeAppDir, 'theme', 'default_200_percent', 'chromium')
fileMap[path.join(braveComponentsDir, 'resources', 'default_100_percent', 'brave')] = path.join(chromeComponentsDir, 'resources', 'default_100_percent', 'chromium')
fileMap[path.join(braveComponentsDir, 'resources', 'default_200_percent', 'brave')] = path.join(chromeComponentsDir, 'resources', 'default_200_percent', 'chromium')
fileMap[path.join(braveAppVectorIconsDir, 'vector_icons', 'brave')] = path.join(chromeAppDir, 'vector_icons', 'brave')
fileMap[path.join(braveResourcesDir, 'settings', 'brave_page_visibility.js')] = path.join(chromeResourcesDir, 'settings', 'brave_page_visibility.js')
fileMap[path.join(braveResourcesDir, 'settings', 'brave_appearance_page')] = path.join(chromeResourcesDir, 'settings', 'brave_appearance_page')

if (process.platform === 'darwin') {
// Copy proper mac app icon for channel to chrome/app/theme/mac/app.icns.
// Each channel's app icons are stored in brave/app/theme/$channel/app.icns.
// With this copying, we don't need to modify chrome/BUILD.gn for this.
fs.copySync(path.join(braveAppDir, 'theme', 'brave', 'mac', config.channel, 'app.icns'),
path.join(chromeAppDir, 'theme', 'brave', 'mac', 'app.icns'))
fileMap[path.join(braveAppDir, 'theme', 'brave', 'mac', config.channel, 'app.icns')] =
Copy link
Member

@simonhong simonhong Sep 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I think these two files (icon and BRANDING) should be handled differently.
When different channel is built from previous, these two files should be copied.
This kinds of file could be candidates for file hash check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it should still work because I'm checking the actual timestamp and not just checking to see which one is newer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see != check - https://github.com/brave/brave-browser/pull/990/files#diff-3deb3f32958bb937ae05c6f3e4abbdf5R132

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess it could still be a problem if you switch channels back and forth because you'll end up with the same timestamp for all of them at some point

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll change to hash

path.join(chromeAppDir, 'theme', 'brave', 'mac', 'app.icns')

// Copy branding file
let branding_file_name = 'BRANDING'
if (config.channel)
branding_file_name = branding_file_name + '.' + config.channel
fs.copySync(path.join(braveAppDir, 'theme', 'brave', branding_file_name), path.join(chromeAppDir, 'theme', 'brave', 'BRANDING'))
fileMap[path.join(braveAppDir, 'theme', 'brave', branding_file_name)] =
path.join(chromeAppDir, 'theme', 'brave', 'BRANDING')
}

for (source in fileMap) {
let sourceFiles = []
const output = fileMap[source]

// get all the files if source if a directory
if (fs.statSync(source).isDirectory()) {
sourceFiles = util.walkSync(source)
} else {
sourceFiles = [source]
}

for (var i in sourceFiles) {
const sourceFile = sourceFiles[i]
let destinationFile = output
if (fs.statSync(destinationFile).isDirectory()) {
destinationFile = path.join(destinationFile, path.basename(sourceFile))
}

// The destination file might be newer when updating chromium so
// we check for an exact match on the timestamp. We use seconds instead
// of ms because utimesSync doesn't set the times with ms precision
if (!fs.existsSync(destinationFile) ||
Math.floor(new Date(fs.statSync(sourceFile).mtimeMs).getTime() / 1000) !=
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ideally we'd use a file hash here, but that might take a while

Math.floor(new Date(fs.statSync(destinationFile).mtimeMs).getTime() / 1000)) {
fs.copySync(sourceFile, destinationFile)
// can't set the date in the past so update the source file
// to match the newly copied destionation file
const date = fs.statSync(destinationFile).mtime
fs.utimesSync(sourceFile, date, date)
console.log(sourceFile + ' copied to ' + destinationFile)
}
}
}
},

Expand Down Expand Up @@ -179,6 +211,17 @@ const util = {
args += arg + '=' + val + ' '
}
return args.replace(/"/g,'\\"')
},

walkSync: (dir, filter = null, filelist = []) => {
fs.readdirSync(dir).forEach(file => {
if (fs.statSync(path.join(dir, file)).isDirectory()) {
filelist = util.walkSync(path.join(dir, file), filter, filelist)
} else if (!filter || filter.call(null, file)) {
filelist = filelist.concat(path.join(dir, file))
}
})
return filelist
}
}

Expand Down
2 changes: 0 additions & 2 deletions scripts/commands.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ program
.option('--official_build <official_build>', 'force official build settings')
.option('--brave_google_api_key <brave_google_api_key>')
.option('--brave_google_api_endpoint <brave_google_api_endpoint>')
.option('--no_branding_update', 'don\'t copy BRANDING to the chrome theme dir')
.option('--channel <target_chanel>', 'target channel to build', /^(beta|dev|nightly|release)$/i, 'release')
.arguments('[build_config]')
.action(build)
Expand All @@ -50,7 +49,6 @@ program
.option('--official_build <official_build>', 'force official build settings')
.option('--brave_google_api_key <brave_google_api_key>')
.option('--brave_google_api_endpoint <brave_google_api_endpoint>')
.option('--no_branding_update', 'don\'t copy BRANDING to the chrome theme dir')
.option('--channel <target_chanel>', 'target channel to build', /^(beta|dev|nightly|release)$/i, 'release')
.arguments('[build_config]')
.action(createDist)
Expand Down