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

Jump to bottom

TI MSP430 cross compiling #58330

Merged
merged 9 commits into from Mar 29, 2019
Merged

TI MSP430 cross compiling #58330

merged 9 commits into from Mar 29, 2019

Conversation

AerialX
Copy link
Contributor

@AerialX AerialX commented Mar 26, 2019 •
edited

Motivation for this change

Enables compiling for MSP430 microcontrollers.

Remaining unsure points
  • Is fetchzip appropriate here or is fetchurl better to use when the archive is likely an archive that won't superficially change?
  • Should msp430GccSupport be included in stdenv or should it just be expected to be added to buildInputs of any packages targeting msp430 devices?
    • For reference, the msp430GccSupport package contains headers, symbols, and linker scripts for the registers and memory layout of each of the hundreds of MSP430 SKUs.
    • Using the -mmcu= flag with gcc will pull in the appropriate ldscript and header, so I'm assuming this should be included in gcc, libc, or stdenv in some way.
  • Is the current approach (symlink wrapper derivation to combine support files with newlib) the correct idiom to use here? Is there some better way to add the support files as additional default cflags/ldflags search paths for stdenv?
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Ericson2314
Copy link
Member

Do you need to use prebuilt binaries for this arch?

@AerialX
Copy link
Contributor Author

AerialX commented Mar 26, 2019 •
edited

As in do I personally need gcc/binutils in the cache? It'd be nice but if I'm the only one who cares I can always set up my own builds to cachix or something.

If "you" as in the world/nixpkgs... These parts are like AVR, the main goal here is just to be able to set up a cross gcc and dev environment - you don't run linux on them and don't expect to run actual packages/binaries either.

@Ericson2314
Copy link
Member

Ok sorry I misread and throught you were extracting non-nix build tarballs when you are just getting more headers.

Still, why isn't the newlib that comes with gcc no good?

@Ericson2314
Copy link
Member

CC @matthewbauer who de-duplicated a lot of the avr stuff. I'm just trying to make sure we don't have any duplication here like we did before.

@AerialX
Copy link
Contributor Author

AerialX commented Mar 27, 2019 •
edited

Still, why isn't the newlib that comes with gcc no good?

so the way it works is for example, gcc -mmcu=msp430f5513 will:

  • pull in msp430f5513.ld as a linker script from library search path
  • define __MSP430F5513__, for which there's a massive #elif in msp430.h that will make it #include "msp430f5513.h".
  • If you do not have these linker scripts in your environment, compilation with the -mmcu= flag will fail due to the missing file.

These headers/ldscripts contain symbols specific to the features and memory map of that specific mcu - there are about 700+ models. These files are not distributed as part of gcc nor newlib, but are a pretty standard part of the dev environment for any project targeting these parts.

newlib+gcc still cover all the standard libc headers and crt etc.

Ericson2314
Ericson2314 reviewed Mar 29, 2019
View reviewed changes
pkgs/top-level/all-packages.nix Outdated
msp430Newlib = callPackage ../development/misc/msp430/newlib.nix { };
msp430NewlibCross = callPackage ../development/misc/msp430/newlib.nix {
inherit (pkgs.buildPackages.xorg) lndir;
newlib = pkgs.newlibCross;
Copy link
Member

Choose a reason for hiding this comment

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

We really shouldn't use pkgs.*Cross in nixpkgs...almost ever. (@matthewbauer I told you this would happen!)

See what the others do with the special stdenv instead. oh it's already stdenvNoCC, that will be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We really shouldn't use pkgs.*Cross in nixpkgs...almost ever.

Hm, it's only being used here because this package is wrapping/replacing it and being used in a place where newlibCross is already being used, should that be done some other way?

Copy link
Member

Choose a reason for hiding this comment

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

Hey I'm really sorry I straight up misread this as pkgsCross.msp430.... Everything you did is fine, though pkgs. is fine to skip too here.

@Ericson2314
Copy link
Member

@AerialX Again sorry for all the paranoid misdirection from my end here. The PR looks good; thank you for contributing it.

@Ericson2314 Ericson2314 merged commit 842b14b into NixOS:master Mar 29, 2019
@AerialX
Copy link
Contributor Author

AerialX commented Mar 30, 2019

np, thanks! I'm just not familiar with the process or idioms of nixpkgs and wanted to make sure I was understanding and explaining everything properly.

@AerialX AerialX deleted the msp430 branch March 30, 2019 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ericson2314 Ericson2314 Ericson2314 left review comments

@matthewbauer matthewbauer matthewbauer approved these changes

@nbp nbp Awaiting requested review from nbp

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@AerialX @Ericson2314 @matthewbauer @GrahamcOfBorg @Janik-Haag