nxpmicro-mfgtools: init at 1.3.191 #91634

bmilanov · 2020-06-26T20:21:22Z

Add nxpmicro-mfgtools to nixpkgs

From the project's homepage 1:

Freescale/NXP I.MX Chip image deploy tools.

The project's only binary and library at the moment is UUU. NXP
might add more in the future, so making the package name:

nxpmicro-mfgtools

instead of:

nxp-uuu

or something similar.

Add a preConfigure step based on bbigras' suggestion.

Motivation for this change

Things done

bbigras

I think it would be cleaner that way.

EDIT: derp, I didn't add the ~~requested~~ suggested changes.

pkgs/applications/science/electronics/nxpmicro-mfgtools/default.nix

bbigras · 2020-06-27T18:01:15Z

You didn't check "Tested using sandboxing". Are you sure you are not using sandboxing? It's enabled by default in NixOS.

You can check by running: nix-shell -p nix-info --run "nix-info -m"

bbigras · 2020-06-27T18:05:58Z

pkgs/applications/science/electronics/nxpmicro-mfgtools/default.nix

+    homepage = https://github.com/NXPmicro/mfgtools;
+    license = licenses.bsd3;
+    maintainers = [ maintainers.bmilanov ];
+    platforms = [ "x86_64-linux" ];


Are you sure it's Linux only? The project's readme says:

The real cross platform. Linux, Windows, MacOS(not test yet)

Maybe try platforms.all.

It says it's cross-platform, but I wanted to add only the platform that I am working on.

Should I add all stated working, or should I add only confirmed platforms?

A bit more info, at:

https://github.com/NXPmicro/mfgtools/releases

NXP publishes compiled binaries, and I can confirm that the one for Windows works.

I don't know what the NixOS policy exactly is on this but maybe it's fine if we put "all" and the ofborg bot doesn't complain while building for all the platforms it supports.

13f3760 fingers crossed

pkgs/applications/science/electronics/nxpmicro-mfgtools/default.nix

bbigras · 2020-06-27T18:16:04Z

pkgs/applications/science/electronics/nxpmicro-mfgtools/default.nix

+  version = "uuu_1.3.191";
+
+  meta = with stdenv.lib; {
+    description = "uuu (Universal Update Utility), mfgtools 3.0";


For brevity, don’t repeat the name of package — just describe what it does

Check https://nixos.org/nixpkgs/manual/#sec-standard-meta-attributes

I think a description of what the program does would be better.

I wonder if it's better to get the contents of the long description in this field. Then think about what to add in the long description itself.

How about this: 5f5d2ee ?

Most of the long description taken from:

https://github.com/NXPmicro/mfgtools/wiki

bmilanov · 2020-06-28T14:09:40Z

Apologies for the multiple commits, I just now saw that I can batch suggestions into one.

bmilanov · 2020-06-28T14:44:44Z

You didn't check "Tested using sandboxing". Are you sure you are not using sandboxing? It's enabled by default in NixOS.

You can check by running: nix-shell -p nix-info --run "nix-info -m"

Seems like I have:

- sandbox: `yes`

bbigras · 2020-06-28T15:32:42Z

Ops I pasted the wrong command sorry.

Ignore the checks about the zenith package.

bbigras · 2020-06-28T15:32:55Z

@GrahamcOfBorg build nxpmicro-mfgtools

bbigras · 2020-06-28T15:39:15Z

bbigras · 2020-06-28T15:43:14Z

I don't know if it matters for new packages but here's the closure size:

❯ nix path-info -S -h /nix/store/fkfi4ainw1iq49zp2jgpmmd0dyqrhixd-nxpmicro-mfgtools-1.3.191
/nix/store/fkfi4ainw1iq49zp2jgpmmd0dyqrhixd-nxpmicro-mfgtools-1.3.191 47.5M

nixos-discourse · 2020-07-01T20:03:35Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/209

danieldk

Thank you for contributing to nixpkgs. I have added some more comments.

Could you squash the commits together? The first commit should add you to maintainers.nix, the second should add the derivation.

pkgs/applications/science/electronics/nxpmicro-mfgtools/default.nix

pkgs/top-level/all-packages.nix

danieldk · 2020-07-02T06:23:24Z

pkgs/applications/science/electronics/nxpmicro-mfgtools/default.nix

+}:
+
+stdenv.mkDerivation rec {
+  pname   = "nxpmicro-mfgtools";


Maybe this should be just be called mfgtools? Not 100% sure though ;).

Hi, thanks for all the feedback!

I am not sure which way's better.

Usually I would add the "nxpmicro-" as I don't know how many mfgtools can pop up in the future (doubt there will be any more), with the assumption that adding "namespaces" later would be harder. I am really tempted to make it only "mfgtools" as it's less text to write.

Your call. I can amend the commit based on what you decide.

Thanks for the changes. There was a small syntax error that caused evaluation to fail, so I fixed that. I also used nixpkgs-fmt to format the derivation.

I think the prefix may be better here. As you say, mfgtools is very generic. I don't think there will be another package with the same name, but with the prefix it's easier to recognize and quickly find.

Whoops, seems like we are force-pushing at the same time. If you only fixed the syntax error, I can push my changes again.

bmilanov · 2020-07-06T18:35:34Z

@danieldk Just after I pushed I saw that you've changed the comma style because of the build error.

I will back off so you can force push to the proper commit.

Add nxpmicro-mfgtools to nixpkgs From the project's homepage [1]: Freescale/NXP I.MX Chip image deploy tools. The project's only binary and library at the moment is UUU. NXP might add more in the future, so making the package name: nxpmicro-mfgtools instead of: nxp-uuu or something similar. [1]: https://github.com/NXPmicro/mfgtools

danieldk

Thanks, looks great now!

Result of nixpkgs-review pr 91634 1

1 package built:

- nxpmicro-mfgtools

Tested binary to work.

bbigras · 2020-07-06T19:08:38Z

Thanks @bmilanov for your contribution and perseverance.

Thanks @danieldk too.

ofborg bot added 8.has: package (new) 11.by: package-maintainer 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Jun 26, 2020

bbigras requested changes Jun 27, 2020

View reviewed changes

bbigras reviewed Jun 27, 2020

View reviewed changes

pkgs/applications/science/electronics/nxpmicro-mfgtools/default.nix Outdated Show resolved Hide resolved

bbigras reviewed Jun 27, 2020

View reviewed changes

pkgs/applications/science/electronics/nxpmicro-mfgtools/default.nix Outdated Show resolved Hide resolved

bbigras reviewed Jun 27, 2020

View reviewed changes

pkgs/applications/science/electronics/nxpmicro-mfgtools/default.nix Outdated Show resolved Hide resolved

bbigras reviewed Jun 27, 2020

View reviewed changes

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 and removed 10.rebuild-darwin: 0 labels Jun 28, 2020

bbigras approved these changes Jun 28, 2020

View reviewed changes

danieldk suggested changes Jul 2, 2020

View reviewed changes

bmilanov force-pushed the submit/add-nxp-mfgtools branch from 13f3760 to b437a30 Compare July 6, 2020 17:45

danieldk force-pushed the submit/add-nxp-mfgtools branch from b437a30 to a27327f Compare July 6, 2020 18:31

bmilanov force-pushed the submit/add-nxp-mfgtools branch from a27327f to 93dbfd8 Compare July 6, 2020 18:32

danieldk force-pushed the submit/add-nxp-mfgtools branch from 93dbfd8 to a27327f Compare July 6, 2020 18:36

maintainers: add bmilanov

859e204

danieldk force-pushed the submit/add-nxp-mfgtools branch from a27327f to cb2ab83 Compare July 6, 2020 18:43

danieldk approved these changes Jul 6, 2020

View reviewed changes

danieldk merged commit f4dce8e into NixOS:master Jul 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nxpmicro-mfgtools: init at 1.3.191 #91634

nxpmicro-mfgtools: init at 1.3.191 #91634

bmilanov commented Jun 26, 2020 •

edited

bbigras left a comment •

edited

bbigras commented Jun 27, 2020

bbigras Jun 27, 2020

bmilanov Jun 28, 2020 •

edited

bbigras Jun 28, 2020

bmilanov Jun 28, 2020

bbigras Jun 27, 2020

bmilanov Jun 28, 2020

bmilanov Jun 28, 2020 •

edited

bmilanov commented Jun 28, 2020

bmilanov commented Jun 28, 2020

bbigras commented Jun 28, 2020 •

edited

bbigras commented Jun 28, 2020

bbigras commented Jun 28, 2020 •

edited

bbigras commented Jun 28, 2020

nixos-discourse commented Jul 1, 2020

danieldk left a comment

danieldk Jul 2, 2020

bmilanov Jul 6, 2020

danieldk Jul 6, 2020

danieldk Jul 6, 2020

bmilanov commented Jul 6, 2020 •

edited

danieldk left a comment

bbigras commented Jul 6, 2020

nxpmicro-mfgtools: init at 1.3.191 #91634

nxpmicro-mfgtools: init at 1.3.191 #91634

Conversation

bmilanov commented Jun 26, 2020 • edited

Motivation for this change

Things done

bbigras left a comment • edited

Choose a reason for hiding this comment

bbigras commented Jun 27, 2020

Choose a reason for hiding this comment

bmilanov Jun 28, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmilanov Jun 28, 2020 • edited

Choose a reason for hiding this comment

bmilanov commented Jun 28, 2020

bmilanov commented Jun 28, 2020

bbigras commented Jun 28, 2020 • edited

bbigras commented Jun 28, 2020

bbigras commented Jun 28, 2020 • edited

Reviewed points

Possible improvements

Comments

bbigras commented Jun 28, 2020

nixos-discourse commented Jul 1, 2020

danieldk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmilanov commented Jul 6, 2020 • edited

danieldk left a comment

Choose a reason for hiding this comment

bbigras commented Jul 6, 2020

bmilanov commented Jun 26, 2020 •

edited

bbigras left a comment •

edited

bmilanov Jun 28, 2020 •

edited

bmilanov Jun 28, 2020 •

edited

bbigras commented Jun 28, 2020 •

edited

bbigras commented Jun 28, 2020 •

edited

bmilanov commented Jul 6, 2020 •

edited