kervinck / gigatron-rom

System, apps and tooling for the Gigatron TTL microcomputer
BSD 2-Clause "Simplified" License
235 stars 80 forks source link

Building ROMv4.rom and dev.rom #117

Closed at67 closed 5 years ago

at67 commented 5 years ago

Clean repo with Python 2.7 and appropriate build tools allows me to build ROMv1.rom, ROMv2.rom, ROMv3.rom and ROMv3y.rom

But not ROMv4.rom or dev.rom

Captures of error logs are attached, I tried adding full paths to SYS_Racer_v1.py in the makefile but that didn't make any difference, (as it shouldn't have because of the exports).

ROMv4_build DEV_build

kervinck commented 5 years ago

Perhaps the export lines aren't universally recognised by make programs.

# Allow application-specific SYS extensions to live in Apps/
export PYTHONPATH=Apps/Loader:Apps/Racer
export PYTHONDONTWRITEBYTECODE=please

I'm using GNU Make 3.81.

(And perhaps it's a good time for Gigatron to say goodbye to python2.7 soon)

kervinck commented 5 years ago

Perhaps it's the colon?

PYTHONPATH Augment the default search path for module files. The format is the same as the shell’s PATH: one or more directory pathnames separated by os.pathsep (e.g. colons on Unix or semicolons on Windows).

at67 commented 5 years ago

Yup, the colon needed to be a semicolon; the ln -sf "$<" "$@" also fails under Windows as obviously ln is not a recognised command under a standard Windows installation, (Windows uses mklink).

When I comment out the export and add the relative paths like this:

        Apps/Racer/SYS_Racer_v1.py\
        Apps/Loader/SYS_Loader.py\

Which is what I tried the first time I encountered the error, it still fails which makes no sense. Anyway there is a simple solution with the semicolon which is good enough, cheers.

at67 commented 5 years ago

I wrapped the export command like this and it seems to work, not sure if it's worth adding to the makefile for Windows users to be honest. It depends on an env variable that should only exist on Windows boxes :/

ifdef OS
export PYTHONPATH=Apps/Loader;Apps/Racer
else
export PYTHONPATH=Apps/Loader:Apps/Racer
endif
kervinck commented 5 years ago

Thanks. It's wise to make an effort to keep it working on Windows, even though I can't test it (which is a guarantee that things will break from time to time. But still..)

What would the mklink command need to look like?

at67 commented 5 years ago

Probably like this:

ifdef OS
gigatron.rom: $(DEV)
    mklink $@ $<
else
gigatron.rom: $(DEV)
    ln -sf "$<" "$@"
endif

I just tested it and it works manually but not from the makefile, mklink makes symbolic links by default and has it's parameters swapped compared to ln.

No idea why it doesn't work in the makefile, but does from the command line; just did some more research: there's a Stackoverflow thread talking about the limitations of mklink used in makefiles with some potential solutions, but none of them worked for me.

https://stackoverflow.com/questions/31232314/mingw32-make-mklink-just-not-getting-along

kervinck commented 5 years ago

Can we use copy on windows instead?

at67 commented 5 years ago

This works, but I had to elevate my command prompt's privileges to Admin, (which is trivial as long as you have Admin rights). Also I have only tested this on Windows10, I doubt it will work on older versions of Windows because of the upgrades that have happened to the command prompt in Win10.

ifdef OS
gigatron.rom: $(DEV)
    copy $< $@
else
gigatron.rom: $(DEV)
    ln -sf "$<" "$@"
endif
kervinck commented 5 years ago

This is my proposal for the colons:

# Allow application-specific SYS extensions to live in Apps/
export PYTHONPATH:=Apps/Loader:Apps/Racer
export PYTHONDONTWRITEBYTECODE:=please

ifdef OS # Windows
 export PYTHONPATH:=$(subst :,;,$(PYTHONPATH))
endif
at67 commented 5 years ago

Just tested it, it works fine.

kervinck commented 5 years ago

Ok, I pushed a change for the semi-colons.

For the other issue, the admin elevation is very annoying for such an unimportant step. Not worth such a blurb yet, in my opinion. Does it have 'cp' perhaps? Or can we create the copy by redirection?

at67 commented 5 years ago

I just tried again in an unelevated command prompt and this time it worked, I've doubled checked and it definitely works now, this is with this snippet:

ifdef OS
gigatron.rom: $(DEV)
    copy $< $@
else
gigatron.rom: $(DEV)
    ln -sf "$<" "$@"
endif

Not sure why it didn't work earlier without an elevated command prompt, I guess PEBKAC P.S. No matter what I try I can't get mklink to do anything at all.

kervinck commented 5 years ago

Ok, I pushed a change: https://github.com/kervinck/gigatron-rom/commit/c2d023db1c6bc83841acc9fd7fa5f87cb69111ba Does it work?

at67 commented 5 years ago

Yup, works perfectly.

kervinck commented 5 years ago

Good! You can close the issue if this will do. At some point later this year I want to migrate everything away from python2.7. I guess by that time things break again and I might need your help.

at67 commented 5 years ago

No problem, closing.