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

feat: #1547 Added pkg license #2212

Closed
wants to merge 4 commits into from

Conversation

Egorikhin
Copy link
Contributor

No description provided.

Copy link
Member

@develar develar left a comment

Choose a reason for hiding this comment

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

Thanks for a new feature.

@@ -70,6 +71,11 @@ export class PkgTarget extends Target {
let distInfo = await readFile(distInfoFile, "utf-8")
const insertIndex = distInfo.lastIndexOf("</installer-gui-script>")
distInfo = distInfo.substring(0, insertIndex) + ` <domains enable_anywhere="${options.allowAnywhere}" enable_currentUserHome="${options.allowCurrentUserHome}" enable_localSystem="${options.allowRootDirectory}" />\n` + distInfo.substring(insertIndex)
if (options.license) {
Copy link
Member

Choose a reason for hiding this comment

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

We should not require explicit option.

  1. use getLicenseFiles util method to autodetect (see other implementations).
  2. disable autodetect of option explicitly set to null — triple equals

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you like me to do something like this?
http://www.cocoabuilder.com/archive/xcode/319021-add-localized-readme-license-etc-in-distribution-package-using-productbuild.html

in your resource folder, add the localized folders, but named English.proj, French.proj, German.proj (they will be renamed as en.proj, fr.proj, de.proj by productbuild) containing your localized Welcome.rtf (for example), and use the --resources option.

Copy link
Member

Choose a reason for hiding this comment

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

At first I think we should add basic implementation, and add localized later. If you want to add localization righ now — great. If not — it is ok too, you are not forced to prepare polished full featured PR.

Thanks for info. Yes, we can automatically creates such files for productbuild since localized licenses supported for DMG/nsis.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order not to make unnecessary pushes, could you please tell me if my approach is right or not:

    if (options.license !== null) {
      const licenseFiles = await getLicenseFiles(this.packager)
      const license = await this.packager.getResource(options.license, ...licenseFiles.map(f => f.file))
      if (license != null) {
        const licensePath = path.join(this.packager.info.projectDir, license)
        distInfo = distInfo.substring(0, insertIndex) + `    <license file="${licensePath}"/>\n` + distInfo.substring(insertIndex)
      }
    }

@Egorikhin
Copy link
Contributor Author

Any comments on my new changes, please?

@develar develar closed this in a0cb477 Oct 30, 2017
@develar
Copy link
Member

develar commented Oct 30, 2017

Released as 19.43.0, thanks.

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.

3 participants