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

PlatformIO not rebuilding symlinked changed files #4316

Closed
1 task done
gsauthof opened this issue Jun 12, 2022 · 10 comments
Closed
1 task done

PlatformIO not rebuilding symlinked changed files #4316

gsauthof opened this issue Jun 12, 2022 · 10 comments

Comments

@gsauthof
Copy link

What kind of issue is this?

  • PlatformIO Core.

Configuration

Operating system: Fedora 35

PlatformIO Version (platformio --version):

PlatformIO Core, version 5.2.4

Platformio was installed via:

python -m venv platformio
source platformio/bin/activate
pip install platformio

Description of problem

Platformio CLI doesn't rebuild changed files when they are symlinked.

This is caused by an SCons bug: MD5-timestamp decider does no longer follow symlinks

Steps to Reproduce

  1. Create a small hello-world project
  2. Create a header in a temporary directory and create a symlink to it in the include subdirectory
  3. Include the symlinked header from - say - src/main.c
  4. run pio run --verbose
  5. change the header
  6. run pio run --verbose again

Actual Results

src/main.c isn't rebuilt

Expected Results

src/main.c is rebuilt

If problems with PlatformIO Build System:

The content of platformio.ini:

[env:pro8MHzatmega328]
platform = atmelavr
board = pro8MHzatmega328
framework = arduino

i.e. pretty vanilla

Source file to reproduce issue:

#include <dummytest.h>

int main() { return 0; }

Additional info

See also my post on the Platformio forum: https://community.platformio.org/t/force-platformio-to-use-modification-time-comparisons-for-determining-whether-a-file-changed/28292

So I'm interested in a work-around, i.e. a way to tell Platformio to simply use the SCons timestamp-newer decider instead of the default md5sum one.

Even if you update the minimal required SCons version in your Platformio Python package dependencies, I still would prefer to use timestamp-newer decider, because I consider the SCons md5sum default as suboptimal and a waste of CPU cycle, in the best case.

Of course, I wouldn't be against Platformio being changed to even use the timestamp-newer decider, by default.

@ivankravets ivankravets added this to the 6.0.3 milestone Jun 13, 2022
@ivankravets ivankravets removed this from the 6.0.3 milestone Jun 13, 2022
@ivankravets
Copy link
Member

The bug is fixed in the latest development version of SCons. See SCons/scons#4121

There are +/- for timestamp decider. See docs for them https://scons.org/doc/production/HTML/scons-user.html#idm46358241264176

If you want to use "make"-like decider, override it in PRE Script

Import("env")

env.Decider("make")

@gsauthof
Copy link
Author

If you want to use "make"-like decider, override it in PRE Script

Sounds great, I had no idea that platformio supports such hooks!

I've tested your snippet and it works well - for non-symlinked files, that is.

That means the SCons issue isn't just limited to the md5sum decider but also affects the 'make' decider! Looking at the SCons patch this is plausible since they also fixed the mtime getter for symlinks.

FWIW, As of 2022-06-20, the latest available SCons release is 4.3.0 - which is from November 2021 and hence doesn't contain the symlink-follow fixes.

Thus, besides configuring the make decider I also 'backported' the mtime fix from upstream to the platformio copy:

--- /home/juser/.platformio/packages/tool-scons/scons-local-4.3.0/SCons/Node/FS.py.orig	2022-06-20 23:56:24.974977949 +0200
+++ /home/juser/.platformio/packages/tool-scons/scons-local-4.3.0/SCons/Node/FS.py	2022-06-20 23:58:26.650017670 +0200
@@ -733,10 +733,7 @@
         return SCons.Node._rexists_map[self._func_rexists](self)
 
     def getmtime(self):
-        if self.islink():
-            st = self.lstat()
-        else:
-            st = self.stat()
+        st = self.stat()
 
         if st:
             return st[stat.ST_MTIME]

And with that (and the make decider), symlinked files are now properly rebuilt with platformio!


For completeness, I just put your snippet into mtime.py and added

extra_scripts = pre:mtime.py

to my platformio.ini

@m1cr0lab
Copy link

It doesn't seem to work anymore on macOS (Big Sur 11.6.7) since the upgrade to 1.6.3.
It used to work with 1.6.2.

pio --version
PlatformIO Core, version 6.1.3

Here is my src folder:

src
├── example.cpp
└── main.cpp -> example.cpp

When I compile the linked file :

; platformio.ini
build_src_filter = -<*> +<example.cpp>

Everything is going well:

Building in debug mode
Compiling .pio/build/examples/src/example.cpp.o
Linking .pio/build/examples/firmware.elf

However, if I compile the symlink:

; platformio.ini
build_src_filter = -<*> +<main.cpp>

Changes in example.cpp are not taken into account:

Building in debug mode
Retrieving maximum program size .pio/build/examples/firmware.elf

Have you encountered this problem as well?

@ivankravets
Copy link
Member

@m1cr0lab does this solution with extra script work for you #4316 (comment) ?

@m1cr0lab
Copy link

I don't think so 😕
I made this change:

# ~/.platformio/packages/tool-scons/scons-local-4.3.0/SCons/Node/FS.py
# line 255-258(+1)
def _copy_func(fs, src, dest):
    shutil.copy2(src, dest)
    st = fs.stat(src)
    # fs.chmod(dest, stat.S_IMODE(st[stat.ST_MODE]) | stat.S_IWRITE)
    fs.chmod(dest, stat.S_IMODE(st.st_mode) | stat.S_IWRITE)

And still nothing at the compilation:

Building in debug mode
Retrieving maximum program size .pio/build/examples/firmware.elf
Checking size .pio/build/examples/firmware.elf

@ivankravets
Copy link
Member

Please re-read my comment.

@m1cr0lab
Copy link

Well... I'm not sure I understand what I'm supposed to do 😁

; platformio.ini
build_src_filter = -<*> +<main.cpp>
extra_scripts = pre:extra_script.py
# extra_script.py (located in the same directory as platformio.ini)
Import("env")

env.Decider("make")

Nothing new:

Building in debug mode
Retrieving maximum program size .pio/build/examples/firmware.elf
Checking size .pio/build/examples/firmware.elf

@m1cr0lab
Copy link

m1cr0lab commented Jul 24, 2022

In fact, if I use the tip given by @gsauthof above:

# ~/.platformio/packages/tool-scons/scons-local-4.3.0/SCons/Node/FS.py
# line 735-745
def getmtime(self):
    # if self.islink():
    #     st = self.lstat()
    # else:
    #     st = self.stat()
    st = self.stat()

    if st:
        return st[stat.ST_MTIME]
    else:
        return None

Then it works:

Building in debug mode
Compiling .pio/build/examples/src/main.cpp.o
Linking .pio/build/examples/firmware.elf
Retrieving maximum program size .pio/build/examples/firmware.elf
Checking size .pio/build/examples/firmware.elf

Thank you all for your help! 🙏

@ivankravets
Copy link
Member

Thanks! We have just patched SCons version in the registry. Could you remove ~/.platformio/packages/tool-scons and ~/.platformio/.cache folders. Does it work now?

@m1cr0lab
Copy link

It works like a charm!
Thanks for everything. 🙏

Tool Manager: Installing platformio/tool-scons @ ~4.40300.0
Downloading  [####################################]  100%          
Unpacking  [####################################]  100%
Tool Manager: tool-scons@4.40300.220725 has been installed!
[ ... ]
Building in debug mode
Compiling .pio/build/examples/src/main.cpp.o
Linking .pio/build/examples/firmware.elf
Retrieving maximum program size .pio/build/examples/firmware.elf
Checking size .pio/build/examples/firmware.elf

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

No branches or pull requests

3 participants