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

makefiles README.md: All programs and Makefiles can run under Windows. #3370

Closed
wants to merge 1 commit into from

Conversation

chrismas9
Copy link
Contributor

Continuation of #3310

py/mkenv.mk: Select whether to add .exe dependant of Target OS instead
of Host OS
This allows Windows port to be cross compiled under Linux and
build micropython.exe to run under Windows. Fixes #3361.

py/mkrules.mk: Fix rule to build mpy-cross / mpy-cross.exe.

windows/Makefile: Add a default CROSS_COMPILE for Windows port.
Move PROG and CROSS_COMPILE before #include mkenv.mk

mpy-cross/Makefile: Move PROG before #include mkenv.mk

tools/mpy_cross_all.py: pass full mpy-cross path/name including .exe
as argument from makefile. Use $(MPY-CROSS) from mkenv.mk.
Convert slash to backslash for Windows cmd.

Document how to install and build with MSYS2 / Mingw under Windows.
Update README.md and windows/README.md for changes above.

With these changes all programs (mpy-cross and micropython.exe) and all
Makefiles will run from a Windows cmd prompt. They don't need to be
run from the MSYS cmd shell. Requires MSYS2 and Mingw to be
installed as documented in the README.md files.

Continuation of micropython#3310

py/mkenv.mk: Select whether to add .exe dependant of Target OS instead
    of Host OS
    This allows Windows port to be cross compiled under Linux and
    build micropython.exe to run under Windows. Fixes micropython#3361.

py/mkrules.mk: Fix rule to build mpy-cross / mpy-cross.exe.

windows/Makefile: Add a default CROSS_COMPILE for Windows port.
                  Move PROG and CROSS_COMPILE before #include mkenv.mk

mpy-cross/Makefile: Move PROG before #include mkenv.mk

tools/mpy_cross_all.py: pass full mpy-cross path/name including .exe
    as argument from makefile. Use $(MPY-CROSS) from mkenv.mk.
    Convert slash to backslash for Windows cmd.

Document how to install and build with MSYS2 / Mingw under Windows.
Update README.md and windows/README.md for changes above.

With these changes all programs (mpy-cross and micropython.exe) and all
    Makefiles will run from a Windows cmd prompt. They don't need to be
    run from the MSYS cmd shell. Requires MSYS2 and Mingw to be
    installed as documented in the README.md files.
@stinos
Copy link
Contributor

stinos commented Oct 16, 2017

It's probably better to split this in two commits: one for the build changes, one for the Readme changes. Also since the Windows port already has a readme, I don't think it's needed to (only partly) duplicate it in the main readme.

# define main target
# PROG is used in py/mkenv.mk. Define first.
# define main target. Compile to run on Host (Linux, OSX or Windows),
# do not include .exe in PROG.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is sort of obvious since the line below doesn't have the .exe ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to document the rule somewhere. It affects whether to cross or native compile. The rules are:

  1. To always compile for Windows target (eg micropython port) add .exe
  2. To compile for the Host environment (eg mpy-cross) don't add .exe.

Is there a better way / place to document this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah now I understand; this is probably the best place to document it, but I only understood what you mean after your comment here. So I'd suggest to reword the comment in the Makefile to make these rules clear.

---------------------------------------

sudo apt-get install gcc-mingw-w64
make
Copy link
Contributor

Choose a reason for hiding this comment

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

This is confusing. If this works, why would you ever use the line below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a default compiler because I want the same method to apply for all environments and it has to be just "make" for Windows which uses gcc.exe, size.exe, etc. But part of my philosophy has been to not change or break existing methods that people are happy with. I'm happy to remove the unnecessary alternative. If someone has scripts, makefiles etc that specify the CC they will still work so I guess it still backward compatible without documenting it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I missed the addition of a default for CROSS_COMPILE. I assume that is ok.

Building using MSYS2 / Mingw on Windows
---------------------------------------

Install MSYS2 64 bit from msys2.org
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this have to be 64bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably not. I will have to dig out an old laptop and test 32 bit. I am trying to simplify the experience for Windows users and not add multiple options. Is anyone still using 32 bit Windows as a development host? Maybe I could just add a one liner about MYS32.

Copy link
Contributor

Choose a reason for hiding this comment

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

You could just install the 32bit version on a 64bit windows, no need to get out some old hardware. Probably if you just say 'Install MSYS2' it's ok, or possibly something like 'Install MSYS2 (32bit or 64bit depending on hardware/preference)'.

---------------------------------------

Install MSYS2 64 bit from msys2.org
Run the installer msys2-x86_64-xxxxxxxx.exe
Copy link
Contributor

@stinos stinos Oct 16, 2017

Choose a reason for hiding this comment

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

Note that just downloading the base.tar.gz or whatever it's compressed like also works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am trying to provide simple instructions for newbies by documenting a method that is tested and easy. I'm not sure that a 12 year old that's just started programming with a micro:bit and a Windows laptop would even know what a tar.gz is. Likewise a self taught Python programmer that only has Windows experience. I went through a lot of pain because I followed instructions on line that had me install a toolchain that included i686-w64-mingw32-gcc but not gcc. So I think its better to document a simple method that works. If experience people want to do it differently they can at their own risk.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. It's just that I once had problems with the installer, don't recall what exactly, so since then I just used the compressed version. But indeed for novices that is one extra possibly hard step.

@pfalcon
Copy link
Contributor

pfalcon commented Oct 16, 2017

Also since the Windows port already has a readme, I don't think it's needed to (only partly) duplicate it in the main readme.

+1

This will install gcc.exe which is identical to i686-w64-mingw32-gcc
If you already have MSYS2 / Mingw installed run "gcc -dumpmachine"
from a command prompt to ensure you have the correct gcc installed.
Response should be "i686-w64-mingw32"
Copy link
Contributor

Choose a reason for hiding this comment

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

Except when you're using the 64bit compiler it's x86_64-w64-mingw32

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should be "i686-w64-mingw32" or "x86_64-w64-mingw32" if you have a 64 bit compiler installed. Either will work

@@ -59,18 +59,28 @@ LD += -m32
endif

MAKE_FROZEN = $(TOP)/tools/make-frozen.py
# allow mpy-cross (for WSL) and mpy-cross.exe (for cygwin) to coexist
# allow mpy-cross (for WSL, Cygwin or Linux) and mpy-cross.exe (for Windows / MSYS2 / mingw) to coexist
Copy link
Contributor

@stinos stinos Oct 16, 2017

Choose a reason for hiding this comment

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

This is not entirely correct: the .exe or not does not depend on the host 'OS" (or emulation layer or whatever it's called), but on the compiler used. So if you'd use gcc on Cygwin (or MSYS2 possibly) you don't want to end up with .exe, but on those same hosts you build with mingw-w64 you do want to end up with .exe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mpy-cross is used while running "make". So it should be compiled to run in the same environment that the makefile is running in.

So if you'd use gcc on Cygwin (or MSYS2 possibly) you don't want to end up with .exe

You won't. All POSIX shells report their own OS name, eg msys. You only get Windows-NT from a raw Windows cmd.exe or a Windows program such as Eclipse. Will change line 62:

# allow mpy-cross (for WSL, MSYS, Cygwin or Linux shell) and mpy-cross.exe (for Windows cmd.exe) to coexist

Copy link
Contributor

Choose a reason for hiding this comment

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

I build under Cygwin using make CROSS_COMPILE=i686-w64-mingw32- and get a .exe.

to Windows. MSYS2 should be installed to provide these gnu utilities.

To use frozen bytecode (.mpy modules) you must build mpy-cross for your host OS.
This requires mingw32 gcc and it dependancies.
Copy link
Contributor

Choose a reason for hiding this comment

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

dependancies -> dependencies. Also, the official name is mingw-w64 so better stick to that: Micropython does not support mingw (also named mingw32) anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

from a command prompt to ensure you have the correct gcc installed.
Response should be "i686-w64-mingw32"

MicroPython can then be built by running make from a Windows command prompt
Copy link
Contributor

Choose a reason for hiding this comment

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

You mean cmd.exe right? How exactly does this work? Does the MSYS2 installer automatcially add the directory containing i686-w64-mingw32-gcc.exe and the likes to the PATH? If so, how would you build a 64bit version where you need the x86_64-w64-mingw32-gcc.exe instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes cmd.exe. I run make from cmd.exe and from an Eclipse makefile project. The path needs to be set. I will do a fresh install and verify if the installer sets the path, and document if necessary. You can still specify an alternative compiler with CROSS_COMPILE=.

@chrismas9
Copy link
Contributor Author

I don't think it's needed to (only partly) duplicate it in the main readme.

Baremetal ports (stm32 and esp8266) have modules which need to be compiled with mpy-cross. You can't build stm32 or esp8266 on Windows without gcc, etc. It's too obscure to expect users to look in the windows port for instructions on setting up.

I think it would be better to keep the Windows setup in the main README and just have a one line reference to it in the windows port. I agree it's better not to duplicate the same info.

@stinos
Copy link
Contributor

stinos commented Oct 16, 2017

Arguably that line is already there as it says the subdirectories above may include READMEs with additional info.. Though there is no explicit mention of the windows posrts so maybe that could be added?

@@ -5,9 +5,12 @@ consider to contribute.


Building on Debian/Ubuntu Linux system
Building on Windows 10 with WSL (Windows Subsystem for Linux) Ubuntu Bash shell
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noticed that if you look at this file rendered on Github this looks meesed up as it becomes one line of bold text, so better reword it.

@chrismas9
Copy link
Contributor Author

It's probably better to split this in two commits: one for the build changes, one for the Readme changes.

I haven't figured out how to handle squashing if I do that. Say I have commit1 for code changes and commit 2 for docs. Then I change the code in commit3. Is it possible to squash not consecutive commits 1 & 3? In TortoiseGIT I open the log, select commits to squash and pick "merge into one commit", but it only works for consecutive commits.

I believe this involves re-ordering commits by rebasing. Is that safe to do after a PR? Any advice or guidance would be appreciated.

@stinos
Copy link
Contributor

stinos commented Oct 16, 2017

Yes it's ok to do that in a PR. Afterwards you will have to use push --force or the TortoiseGit equivalent. Which is ok since you're working in your own personal branch.

Actually what I'd do here is first do a mixed resest to the parent commit (something like reset --mixed 829c329), so the commit(s) is/are gone and you are only left with the code changes. Then create new commits. Not sure, but that seems less work here.

If you already have commits and want to keep them anyway, create new commits then do interactive rebase, in which you can choose to squash a previous commit, or 'fixup' which is same as squash but without editing the commit message. So yes you can make one commit out of commit 1 & 3. I have not used rebase in TortoiseGit though.

@chrismas9
Copy link
Contributor Author

@stinos Thanks for taking the time to review this, and for your good advise. I will do the clean up tomorrow.

@@ -6,6 +6,7 @@
argparser = argparse.ArgumentParser(description="Compile all .py files to .mpy recursively")
argparser.add_argument("-o", "--out", help="output directory (default: input dir)")
argparser.add_argument("--target", help="select MicroPython target config")
argparser.add_argument("--mpy_cross", help="full path/name of the mpy-cross compiler: $(MPY-CROSS) in mkenv.mk")
Copy link
Contributor

Choose a reason for hiding this comment

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

The option name should be "--mpy-cross". The help should be "full path/name of the mpy-cross tool" (extra adds confusion).

@@ -31,8 +32,11 @@
out_dir = os.path.dirname(out_fpath)
if not os.path.isdir(out_dir):
os.makedirs(out_dir)
cmd = "mpy-cross -v -v %s -s %s %s -o %s" % (TARGET_OPTS.get(args.target, ""),
cmd = '"' + args.mpy_cross + '" -v -v %s -s %s %s -o %s' % (TARGET_OPTS.get(args.target, ''),
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens here and below if --mpy-cross is not passed?

Copy link
Contributor

@pfalcon pfalcon left a comment

Choose a reason for hiding this comment

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

Please separate changes to mpy_cross_all.py to a separate commit (and separate PR, if you want to have it merged before the rest of stuff is reviewed, which may take long, long time).

@pfalcon
Copy link
Contributor

pfalcon commented Oct 16, 2017

-1 for spurious renaming of PROG to PROG_BASE. -1 for adding anything to the top README.

tannewt added a commit to tannewt/circuitpython that referenced this pull request Sep 11, 2020
add bigram compression to makeqstrdata (save ~100 bytes on trinket m0 de_DE)
@dpgeorge
Copy link
Member

dpgeorge commented May 4, 2021

@chrismas9 there are some worthwhile changes here but it's a bit stale now because the code has changed since this PR was opened. So I'll close this, but if you want to get the changes in then I suggest to make smaller, self-contained commits/PRs. In particular separate the change to the build(s) from the documentation changes.

@dpgeorge dpgeorge closed this May 4, 2021
dpgeorge pushed a commit to dpgeorge/micropython that referenced this pull request Jul 7, 2021
dpgeorge pushed a commit to dpgeorge/micropython that referenced this pull request Jul 8, 2021
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.

micropython.exe: Building with MINGW fails
4 participants