Issue 7020 - publish gecko with Node.js

automation/target/gecko.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 21 matching lines @@
   parser.addArgument(["-p", "--package"], {required: true});
   parser.addArgument(["-c", "--credentials"], {required: true});
   parser.addArgument(["-t", "--target"], {defaultValue: "dist"});
 };
 
 exports.run = function(args)
 {
   let appIdFromPython = new Promise((resolve, reject) =>
   {
     exec(
       "python -c \"" +

[kzar, 2018/10/15 12:42:58]

Maybe we should parse the file using JavaScript instead?

[tlucas, 2018/10/15 14:12:39]

Originally, we would have to support inheritance in .ini-format files to
accomplish that. Unfortunately, no .ini-parsers for nodejs support that feature
(and in the buildtools, we added it ourselves) - and for the time being
Sebastian and i agreed, that "a little inline python" is acceptable, before we
implement the full feature in JS.

[kzar, 2018/10/15 14:46:10]

Alright, fair enough. Personally, I'd vote for just switching it to JSON or
sometihng that's easier to parse from Nodejs, but I guess we can worry about
that later!

         "from buildtools.chainedconfigparser import ChainedConfigParser; " +
         "p = ChainedConfigParser(); " +
         "p.read('metadata.gecko'); " +
         "print p.get('general', 'app_id_devbuild')\"",
       (error, stdout, stderr) =>
       {
         if (error)
         {
           console.error(stderr);
           reject(error);
@@ skipping 17 matching lines @@
     return signAddon({
       xpiPath: args.package,
       version,
       apiKey: auth["AMO_KEY"],
       apiSecret: auth["AMO_SECRET"],
       channel: "unlisted",
       id: appId
     });
   }).then(result =>
   {
+    // signAddon writes failure reasons directly to the shell, so we don't have
+    // to take care about logging the error messages.
     if (!result.success)
       process.exit(1);

[kzar, 2018/10/15 12:42:58]

Should we instead throw an exception here, so it's caught and logged below?

[tlucas, 2018/10/15 14:12:39]

Sebastian also asked if we could log something here:

https://codereview.adblockplus.org/29901591/diff/29901592/automation/builds/g...
-> the last comment section.

[kzar, 2018/10/15 14:46:10]

I see, that's a little misleading but it's not your fault, but the fault of this
sign-addon library. Perhaps add a small comment above process.exit(1) to mention
that signAddon logs the failure reason to the console?

[tlucas, 2018/10/16 08:42:39]

Done.

 
     let fullName = result.downloadedFiles[0];
     let newName = path.join(args.target, path.basename(fullName));
 
     return mvFileAsync(fullName, newName, {mkdirp: true});
   }).catch(err =>
   {
     console.error(err);
     process.exit(1);
   });
 };