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

Same codebase on multiple client operating systems #310

Open
WasabiFan opened this issue Nov 2, 2019 · 7 comments
Open

Same codebase on multiple client operating systems #310

WasabiFan opened this issue Nov 2, 2019 · 7 comments

Comments

@WasabiFan
Copy link
Contributor

The templates for the SConscript file and a variety of other files are intentionally designed to produce paths (and, in some sporadic cases, newlines) which are specific to the OS the files are generated on.

I've been urged in a few other issues to check in the files that modm generates, but as a result if I generated my files on a Windows machine and then cloned the repo on a Linux machine, the project is unusable.

In my case, the following files are different between the two:

        modified:   modm/SConscript
        modified:   modm/ext/cmsis/device/stm32f427xx.h
        modified:   modm/ext/cmsis/device/system_stm32f4xx.h
        modified:   modm/ext/printf/printf.source
        modified:   modm/openocd.cfg
        modified:   modm/scons/site_tools/template.py
        modified:   modm/src/modm/architecture/interface.hpp
        modified:   modm/src/modm/math.hpp
        modified:   modm/src/modm/platform.hpp
        modified:   modm/src/modm/processing.hpp

What am I supposed to do about this?

@salkinium
Copy link
Member

What am I supposed to do about this?

Weep. Windows support is such a pita.

Can you dump a diff somewhere, I want to know what paths are generated wrongly.
I know that SConscript is special, but why the other files?

@WasabiFan
Copy link
Contributor Author

Yeah, I can grab a diff in a few hours.

I didn't look too closely so I'm not sure how many of them are paths and how many are line endings; I know the SConscript had the path issue and one of the other files had swapped line endings.

For the paths in SConscript, I'd say the most obvious solution would be to have it do the path comversion at runtime: Python has functions to handle that if needed, so you can generate a single script that works on both and uses only a single path string representation in the source.

@WasabiFan
Copy link
Contributor Author

Here's a diff from running lbuild on Linux after originally generating the files on Windows. Pretty much all of it could be fixed by standardizing on linux-style paths, I think, since forward slashes work on both systems while backslashes only work in that way on Windows.

diff.txt

@salkinium
Copy link
Member

Pretty much all of it could be fixed by standardizing on linux-style paths

Completely agreed!

There was a reason why the SConscript had to be this way, probably because Python doesn't properly handle Windows paths.

-    abspath(r"ext\cmsis\core"),
-    abspath(r"ext\cmsis\device"),
+    abspath(r"ext/cmsis/core"),
+    abspath(r"ext/cmsis/device"),

That's probably because the submodules differ. ST changes the permissions every now and again and I didn't catch on to it soon enough.

diff --git a/mcb-2019-2020-project/modm/ext/cmsis/device/stm32f427xx.h b/mcb-2019-2020-project/modm/ext/cmsis/device/stm32f427xx.h
old mode 100644
new mode 100755
diff --git a/mcb-2019-2020-project/modm/ext/cmsis/device/system_stm32f4xx.h b/mcb-2019-2020-project/modm/ext/cmsis/device/system_stm32f4xx.h
old mode 100644
new mode 100755

Newlines? Encoding? Wat?

--- a/mcb-2019-2020-project/modm/ext/printf/printf.source
+++ b/mcb-2019-2020-project/modm/ext/printf/printf.source

That's just weird? Maybe an ordering issue?

diff --git a/mcb-2019-2020-project/modm/src/modm/architecture/interface.hpp b/mcb-2019-2020-project/modm/src/modm/architecture/interface.hpp
index cc3c7c3..cfa8163 100644
--- a/mcb-2019-2020-project/modm/src/modm/architecture/interface.hpp
+++ b/mcb-2019-2020-project/modm/src/modm/architecture/interface.hpp
@@ -14,6 +14,17 @@
 
 #include <stdint.h>
 
+#include "interface/accessor.hpp"
+#include "interface/accessor_flash.hpp"
+#include "interface/accessor_ram.hpp"
+#include "interface/assert.hpp"
+#include "interface/atomic_lock.hpp"
+#include "interface/clock.hpp"
+#include "interface/delay.hpp"
+#include "interface/gpio.hpp"
+#include "interface/interrupt.hpp"
+#include "interface/memory.hpp"
+#include "interface/register.hpp"
 #include "interface/peripheral.hpp"
 
 #endif	// MODM_INTERFACE_HPP
\ No newline at end of file

Ok, this is just plain stupid, lbuild exposes the build log of the children modules to generate this list of header files and returns the native Python path for the platform.

diff --git a/mcb-2019-2020-project/modm/src/modm/math.hpp b/mcb-2019-2020-project/modm/src/modm/math.hpp
index 802dc9b..88eb490 100644
--- a/mcb-2019-2020-project/modm/src/modm/math.hpp
+++ b/mcb-2019-2020-project/modm/src/modm/math.hpp
@@ -12,14 +12,14 @@
 #ifndef MODM_MATH_HPP
 #define	MODM_MATH_HPP
 
-#include "math\tolerance.hpp"
-#include "math\units.hpp"
-#include "math\utils.hpp"
-#include "math\utils\arithmetic_traits.hpp"
-#include "math\utils\bit_constants.hpp"
-#include "math\utils\bit_operation.hpp"
-#include "math\utils\crc32.hpp"
-#include "math\utils\endianness.hpp"
-#include "math\utils\misc.hpp"
-#include "math\utils\operator.hpp"
+#include "math/tolerance.hpp"
+#include "math/units.hpp"
+#include "math/utils.hpp"
+#include "math/utils/arithmetic_traits.hpp"
+#include "math/utils/bit_constants.hpp"
+#include "math/utils/bit_operation.hpp"
+#include "math/utils/crc32.hpp"
+#include "math/utils/endianness.hpp"
+#include "math/utils/misc.hpp"
+#include "math/utils/operator.hpp"
 #endif	// MODM_MATH_HPP
\ No newline at end of file

Realistically I need a Windows CI to fix this otherwise I'm just going to break Windows support again and again.

@salkinium
Copy link
Member

salkinium commented Nov 2, 2019

That's just weird? Maybe an ordering issue?

Oh, this is just gold 🤦‍♂:

headers = env.generated_local_files(filterfunc=
        lambda path: re.match(r"interface/.*\.hpp", path))

@WasabiFan
Copy link
Contributor Author

Realistically I need a Windows CI to fix this otherwise I'm just going to break Windows support again and again.

I very much like that idea. Both to run a build in the first place, and ideally to try comparing a Linux build and a Windows build to make sure they have the same output. (although, that last one would require a service with pipelines and artifacts; not sure if the project has that right now.)

@salkinium
Copy link
Member

I've added a partial fix to these problems in 457e815. This at least fixes the backslash issue in the aggregate header files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants