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

Add "binary.tag" option #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add "binary.tag" option #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 7 additions & 6 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,18 @@ npm install -g node-pre-gyp-github
```

## Configuration
This module is intended to be used with node-pre-gyp. Therefore, be sure to configure and install node-pre-gyp first. After having done that, within **```package.json```** update the ```binary``` properties ```host``` and ```remote_path``` so it matches the following format:
This module is intended to be used with node-pre-gyp. Therefore, be sure to configure and install node-pre-gyp first. After having done that, within **```package.json```** update the `binary` section properties so they match the following format:

```
"host": "https://github.com/[owner]/[repo]/releases/download/",
"remote_path": "{version}"
"host": "https://github.com",
"remote_path": "/[owner]/[repo]/releases/download/",
"tag": "{version}"
```

Be sure to replace ```[owner]```, ```[repo]```, with actual values,
but DO NOT replace ```{version}``` with actual version.
Be sure to replace `[owner]`, `[repo]`, with actual values,
but DO NOT replace `{version}` with actual version.

***WARNING: Variable substitutions are not supported on the ```host``` property and on the ```remote_path``` only ```{version}``` placeholder is supported. The value of ```remote_path``` after substitution will become a release tag name. Do not use [forbidden git tag characters](https://git-scm.com/docs/git-check-ref-format) for ```version``` and ```remote_path``` properties.***
***WARNING: Variable substitutions are not supported on the `host` or `remote_path` property. Only `{version}` placeholder is supported for the `tag` property. The value of `tag` after substitution will become a release tag name. Do not use [forbidden git tag characters](https://git-scm.com/docs/git-check-ref-format) in your `package.version` or `package.binary.tag` properties.***

Within GitHub, create a new authorization:

Expand Down
54 changes: 35 additions & 19 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ NodePreGypGithub.prototype.release = {};
NodePreGypGithub.prototype.stage_dir = path.join(cwd,"build","stage");

NodePreGypGithub.prototype.init = function() {
var ownerRepo, hostPrefix;
var ownerRepo, hostPrefix, pathPrefix;

this.package_json = JSON.parse(fs.readFileSync(path.join(cwd,'package.json')));

if(!this.package_json.repository || !this.package_json.repository.url){
throw new Error('Missing repository.url in package.json');
}
Expand All @@ -47,17 +47,26 @@ NodePreGypGithub.prototype.init = function() {
}
else throw new Error('A correctly formatted GitHub repository.url was not found within package.json');
}

hostPrefix = 'https://github.com/' + this.owner + '/' + this.repo + '/releases/download/';

hostPrefix = 'https://github.com'
pathPrefix = '/' + this.owner + '/' + this.repo + '/releases/download/';
if(!this.package_json.binary || 'object' !== typeof this.package_json.binary || 'string' !== typeof this.package_json.binary.host){
throw new Error('Missing binary.host in package.json');
}
else if (this.package_json.binary.host.substr(0, hostPrefix.length) !== hostPrefix){
throw new Error('binary.host in package.json should begin with: "' + hostPrefix + '"');
}
Copy link
Owner

Choose a reason for hiding this comment

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

I was going to ask you to split the above else if statement into two, like so:

else if (!this.package_json.binary.tag && this.package_json.binary.host.substr(0, hostPrefix.length) !== hostPrefix){
  		throw new Error('binary.host in package.json should begin with: "' + hostPrefix + '" when binary.tag is not used.');
  	}
else if (this.package_json.binary.tag && 'string' === typeof this.package_json.binary.tag && this.package_json.binary.host.substr(0, hostPrefix.length) !== hostPrefix){
  		throw new Error('binary.host in package.json should be equal to: "' + hostPrefix + '"');
  	}

but now I am pondering whether it makes sense to try to make this backwards compatible with the old format. What are your thoughts? Regardless, I will be making this a MAJOR version change so I'm not sure there is a point to make it backwards compatible.


else if('string' == typeof this.package_json.binary.tag ) {
if (this.package_json.binary.remote_path.substr(0, pathPrefix.length) !== pathPrefix){
throw new Error('binary.remote_path in package.json should begin with: "' + pathPrefix + '" when using binary.tag');
}
}
else if(! this.package_json.binary.host.match(pathPrefix)) {
throw new Error('binary.host in package.json should contain: "' + pathPrefix + '"');
}

this.github.headers = {"user-agent": (this.package_json.name) ? this.package_json.name : "node-pre-gyp-github"}; // GitHub is happy with a unique user agent

};

NodePreGypGithub.prototype.authenticate_settings = function(){
Expand All @@ -80,13 +89,13 @@ NodePreGypGithub.prototype.createRelease = function(args, callback) {
'draft': true,
'prerelease': false
};

Object.keys(args).forEach(function(key) {
if(args.hasOwnProperty(key) && options.hasOwnProperty(key)) {
options[key] = args[key];
}
});

this.github.authenticate(this.authenticate_settings());
this.github.releases.createRelease(options, callback);
};
Expand All @@ -110,9 +119,9 @@ NodePreGypGithub.prototype.uploadAssets = function(){
consoleLog("Stage directory path: " + path.join(this.stage_dir));
fs.readdir(path.join(this.stage_dir), function(err, files){
if(err) throw err;

if(!files.length) throw new Error('No files found within the stage directory: ' + this.stage_dir);

files.forEach(function(file){
asset = this.release.assets.filter(function(element, index, array){
return element.name === file;
Expand Down Expand Up @@ -141,18 +150,25 @@ NodePreGypGithub.prototype.publish = function(options) {
'repo': this.repo
}, function(err, data){
var release;

if(err) throw err;


// prefer `binary.tag` if present, before remote_path - see https://github.com/bchr02/node-pre-gyp-github/issues/18
if (this.package_json.binary.tag) {
options.tag_name = this.package_json.binary.tag.replace(/\{version\}/g, this.package_json.version);
// stage_dir is still based on remote_path, not tag:
this.stage_dir = path.join(this.stage_dir, (this.package_json.binary.remote_path || ''));
}
// when remote_path is set expect files to be in stage_dir / remote_path after substitution
if (this.package_json.binary.remote_path) {
// {version} substitution only takes place in `remote_path` if `package_json.binary.tag` is not present:
else if (this.package_json.binary.remote_path) {
options.tag_name = this.package_json.binary.remote_path.replace(/\{version\}/g, this.package_json.version);
this.stage_dir = path.join(this.stage_dir, options.tag_name);
} else {
// This is here for backwards compatibility for before binary.remote_path support was added in version 1.2.0.
options.tag_name = this.package_json.version;
}

release = (function(){ // create a new array containing only those who have a matching version.
if(data) {
data = data.filter(function(element, index, array){
Expand All @@ -162,13 +178,13 @@ NodePreGypGithub.prototype.publish = function(options) {
}
else return [];
}.bind(this))();

this.release = release[0];

if(!release.length) {
this.createRelease(options, function(err, release) {
if(err) throw err;

this.release = release;
if (release.draft) {
consoleLog('Release ' + release.tag_name + " not found, so a draft release was created. YOU MUST MANUALLY PUBLISH THIS DRAFT WITHIN GITHUB FOR IT TO BE ACCESSIBLE.");
Expand All @@ -185,4 +201,4 @@ NodePreGypGithub.prototype.publish = function(options) {
}.bind(this));
};

module.exports = NodePreGypGithub;
module.exports = NodePreGypGithub;
46 changes: 23 additions & 23 deletions test/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ var reset_index = function(index_string_ref) {
var reset_mocks = function() {
process.env.NODE_PRE_GYP_GITHUB_TOKEN = "secret";
fs = reset_index('fs');
fs.readFileSync = function(){return '{"name":"test","version":"0.0.1","repository": {"url":"git+https://github.com/test/test.git"},"binary":{"host":"https://github.com/test/test/releases/download/","remote_path":"{version}"}}';};
fs.readFileSync = function(){return '{"name":"test","version":"0.0.1","repository": {"url":"git+https://github.com/test/test.git"},"binary":{"host":"https://github.com","remote_path":"/test/test/releases/download/","tag":"{version}"}}';};
index.stage_dir = stage_dir;
index.github.authenticate = function(){};
index.github.releases.listReleases = function(options, cb){
Expand All @@ -28,9 +28,9 @@ if(!process.env.COVERALLS_SERVICE_NAME) console.log('To post to coveralls.io, be
if(!process.env.COVERALLS_REPO_TOKEN) console.log('To post to coveralls.io, be sure to set COVERALLS_REPO_TOKEN environment variable');

describe("Publishes packages to GitHub Releases", function() {

describe("Publishes without an error under all options", function() {

it("should publish a non-draft release without an error", function() {
var options = {'draft': false, 'verbose': false};
reset_mocks();
Expand All @@ -42,7 +42,7 @@ describe("Publishes packages to GitHub Releases", function() {
};
expect(function(){ index.publish(options); }).to.not.throw();
});

it("should publish a draft release without an error", function() {
var options = {'draft': true, 'verbose': false};
reset_mocks();
Expand All @@ -51,46 +51,46 @@ describe("Publishes packages to GitHub Releases", function() {
};
expect(function(){ index.publish(options); }).to.not.throw();
});

});

describe("Throws an error when node-pre-gyp-github is not configured properly", function() {

it("should throw an error when missing repository.url in package.json", function() {
var options = {'draft': true, 'verbose': false};
reset_mocks();
fs.readFileSync = function(){return '{}';};
expect(function(){ index.publish(options); }).to.throw("Missing repository.url in package.json");
});

it("should throw an error when a correctly formatted GitHub repository.url is not found in package.json", function() {
var options = {'draft': true, 'verbose': false};
reset_mocks();
fs.readFileSync = function(){return '{"repository": {"url":"bad_format_url"}}';};
expect(function(){ index.publish(options); }).to.throw("A correctly formatted GitHub repository.url was not found within package.json");
});

it("should throw an error when missing binary.host in package.json", function() {
var options = {'draft': true, 'verbose': false};
reset_mocks();
fs.readFileSync = function(){return '{"repository": {"url":"git+https://github.com/test/test.git"}}';};
expect(function(){ index.publish(options); }).to.throw("Missing binary.host in package.json");
});

it("should throw an error when binary.host does not begin with the correct url", function() {
var options = {'draft': true, 'verbose': false};
reset_mocks();
fs.readFileSync = function(){return '{"repository": {"url":"git+https://github.com/test/test.git"},"binary":{"host":"bad_format_binary"}}';};
expect(function(){ index.publish(options); }).to.throw(/^binary.host in package.json should begin with:/i);
});

it("should throw an error when the NODE_PRE_GYP_GITHUB_TOKEN environment variable is not found", function() {
var options = {'draft': true, 'verbose': false};
reset_mocks();
process.env.NODE_PRE_GYP_GITHUB_TOKEN = "";
expect(function(){ index.publish(options); }).to.throw("NODE_PRE_GYP_GITHUB_TOKEN environment variable not found");
});

it("should throw an error when github.releases.listReleases returns an error", function() {
var options = {'draft': true, 'verbose': false};
reset_mocks();
Expand All @@ -99,7 +99,7 @@ describe("Publishes packages to GitHub Releases", function() {
};
expect(function(){ index.publish(options); }).to.throw('listReleases error');
});

it("should throw an error when github.releases.createRelease returns an error", function() {
var options = {'draft': true, 'verbose': false};
reset_mocks();
Expand All @@ -111,7 +111,7 @@ describe("Publishes packages to GitHub Releases", function() {
};
expect(function(){ index.publish(options); }).to.throw('createRelease error');
});

it("should throw an error when the stage directory structure is missing", function() {
var options = {'draft': true, 'verbose': false};
reset_mocks();
Expand All @@ -120,7 +120,7 @@ describe("Publishes packages to GitHub Releases", function() {
};
expect(function(){ index.publish(options); }).to.throw('readdir Error');
});

it("should throw an error when there are no files found within the stage directory", function() {
var options = {'draft': true, 'verbose': false};
reset_mocks();
Expand All @@ -129,7 +129,7 @@ describe("Publishes packages to GitHub Releases", function() {
};
expect(function(){ index.publish(options); }).to.throw(/^No files found within the stage directory:/i);
});

it("should throw an error when a staged file already exists in the current release", function() {
var options = {'draft': true, 'verbose': false};
reset_mocks();
Expand All @@ -141,23 +141,23 @@ describe("Publishes packages to GitHub Releases", function() {
};
expect(function(){ index.publish(options); }).to.throw(/^Staged file .* found but it already exists in release .*. If you would like to replace it, you must first manually delete it within GitHub./i);
});

it("should throw an error when github.releases.uploadAsset returns an error", function() {
var options = {'draft': true, 'verbose': false};
reset_mocks();
fs.readdir = function(filename, cb) {
cb(null,["filename"]);
};
index.github.releases.uploadAsset = function(cfg,cb){
cb(new Error('uploadAsset error'));
cb(new Error('uploadAsset error'));
};
expect(function(){ index.publish(options); }).to.throw("uploadAsset error");
});

});

describe("Verify backwords compatible with any breaking changes made within the same MINOR version.", function() {

it("should publish even when package.json's binary.remote_path property is not provided and instead the version is hard coded within binary.host", function() {
var options = {'draft': false, 'verbose': false};
reset_mocks();
Expand All @@ -170,6 +170,6 @@ describe("Publishes packages to GitHub Releases", function() {
};
expect(function(){ index.publish(options); }).to.not.throw();
});

});
});
});