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

Automatically build UI before compiling #3598

Merged
merged 4 commits into from
Jan 9, 2024
Merged

Automatically build UI before compiling #3598

merged 4 commits into from
Jan 9, 2024

Conversation

w00000dy
Copy link
Contributor

This will automatically run npm run build before compiling.

Now we don't have to manually run npm run build anymore.

@blazoncek
Copy link
Collaborator

Is it possible to skip "npm run build" when building for multiple environments?
I have about 20 environments that build regularly, "npm run build" is only need once.

@w00000dy
Copy link
Contributor Author

w00000dy commented Dec 20, 2023

I think the best way to accomplish this is to update cdata.js so that the script only updates the C array when the file has changed.
In PlatformIO, a script defined in the platformio.ini file is executed for each environment when you compile multiple environments at once. However, I think there is no built-in function in PlatformIO to run the script only once when compiling multiple environments.

@w00000dy
Copy link
Contributor Author

@blazoncek If you want I could modify cdata.js to do this.
Just let me know if you would like cdata.js to be modified like this and if so, I will start working on it.

@blazoncek
Copy link
Collaborator

IDK. I do not understand that stuff enough to lend any guidance.
If it cannot be run just once I might prefer current state requiring manual run.

@w00000dy
Copy link
Contributor Author

Okay, I'll just continue developing a first prototype. Once it's ready, you'll have the opportunity to test it. In my opinion, this approach offers numerous advantages and I see no disadvantages. 😄

In the meanwhile I'll convert this PR to a draft.

@w00000dy w00000dy marked this pull request as draft December 21, 2023 21:36
@w00000dy
Copy link
Contributor Author

w00000dy commented Jan 3, 2024

Sorry that nothing has happened here for so long. I didn't get around to continuing here because of the holidays, Christmas and New Year. I just wanted to let you know that I haven't forgotten this. 😃

@w00000dy
Copy link
Contributor Author

w00000dy commented Jan 7, 2024

Now, when running npm run build, a file cdataCache.json gets created that stores the last time the contents of a file were modified and the size of the file that was used for building. Then, before building, it is checked if mtime is newer or if the size has changed.
The mtime changes when the content of a file is changed. However, it could theoretically be possible for a file to be changed and then reset to its original state within the resolution of the mtime timestamp. In this case, the mtime would not change, even though the file had actually changed.
Checking the size helps to recognize such cases. If the size of the file changes, it is very likely that the content of the file has been changed. Therefore, checking both the mtime and the size provides a stronger guarantee that the file has not been changed than checking only the mtime.

I also added an option to always build even if it is cached if you use the command-line parameters -f or --force.
So you can run node .\tools\cdata.js -f or npm run build -- -f to always build the files. I can't see when you would ever need this, but you can use it if you want. 😄

Let me know what you think!

@w00000dy w00000dy marked this pull request as ready for review January 7, 2024 21:58
@blazoncek
Copy link
Collaborator

The best solutions come when people are hard pressed while being constrained.
Good work!

@dosipod
Copy link
Contributor

dosipod commented Jan 9, 2024

@WoodyLetsCode Hi, I am mostly compiling on gh and I would like very much to use the same way for automatic build of ui changes without using VSC , is that doable in the current state . I have tried few days ago but could not do it on github but was able to do the same on VSC .

@w00000dy
Copy link
Contributor Author

w00000dy commented Jan 9, 2024

@dosipod How do you compile it on github?

@dosipod
Copy link
Contributor

dosipod commented Jan 9, 2024

@dosipod How do you compile it on github?

@WoodyLetsCode What i do is clone or import the repo and enable github actions ( this part works and I can compile just fine with PlatformIO CI ) ,
And for this case just like in your repo all i did is :
I added build_ui.py under pio-scripts and the line pre:pio-scripts/build_ui.py in platformio.ini
Did i miss anything ? I also tried to mess with node.js compile from github pages but did not go anywhere with that . I am still on 101 github know how so please excuse any mess

@w00000dy
Copy link
Contributor Author

w00000dy commented Jan 9, 2024

@dosipod I'm still not sure where you compile it. In VS Code or in Github Codespaces or somewhere else?
The steps you took seem to be correct, but I'm wondering why you don't just clone my repository or just the pio branch, because then you can be sure you haven't done anything wrong.
I also do not understand what you did with github pages and why you need them here.
Maybe you can tell me what exactly is not working for you or what you expect to happen. Maybe it would also help to share screenshots, code, ... to better understand your problem.

@blazoncek blazoncek merged commit c4d214f into wled:0_15 Jan 9, 2024
12 checks passed
@blazoncek
Copy link
Collaborator

@WoodyLetsCode there's something very wrong with this.

npm run build --force
npm WARN using --force Recommended protections disabled.

> wled@0.15.0-a0 build
> node tools/cdata.js

Skipping wled00/html_ui.h as it is cached
Skipping wled00/html_pixart.h as it is cached
Skipping wled00/html_cpal.h as it is cached
Skipping wled00/html_pxmagic.h as it is cached
Skipping wled00/html_settings.h as all files are cached
Skipping wled00/html_other.h as all files are cached

Now I cannot rebuild any file unless I remove cache file.

@blazoncek
Copy link
Collaborator

It is interesting to note, that, when doing branch merge, it will not detect correctly file change when there is merge conflict in .h file.

@w00000dy
Copy link
Contributor Author

w00000dy commented Jan 9, 2024

@blazoncek You must add --force to the node .\tools\cdata.js command. So you could use node .\tools\cdata.js --force . Because you use the npm run build command to run node .\tools\cdata.js, you have to add -- before the parameter so that npm knows that the parameter is for the node .\tools\cdata.js command.

Basically you have to use npm run build -- --force instead of npm run build --force.

@w00000dy
Copy link
Contributor Author

w00000dy commented Jan 9, 2024

It is interesting to note, that, when doing branch merge, it will not detect correctly file change when there is merge conflict in .h file.

Now that this is merged, we could add the .h files to .gitignore so that we no longer have merge conflicts with these files.

@w00000dy
Copy link
Contributor Author

w00000dy commented Jan 9, 2024

I will make a new PR for this

@blazoncek
Copy link
Collaborator

Basically you have to use npm run build -- --force instead of npm run build --force.

Ah. Ok, I didn't know that.

@w00000dy
Copy link
Contributor Author

w00000dy commented Jan 9, 2024

@blazoncek I opened #3666. This should also fix the problem you encountered.

@dosipod
Copy link
Contributor

dosipod commented Jan 10, 2024

@WoodyLetsCode I will give it another go and if need be will post detailed info on your repo so not to chat here too much , my end goal is to actually just demo the auto ui when compiling on gh for others guys but not really after doing ui changes per say .[ just noticed i can not post on your repo so here is a quick video of how i enable GH compile , i hope blaz does not mind we chat in here and not sure you are on discord so we could talk there ]
How_to_complie_on_gh

@dosipod
Copy link
Contributor

dosipod commented Jan 10, 2024

@WoodyLetsCode I can see the auto ui is working on gh when i cloned your repo so must be something with my old repo, will sort it out
https://github.com/dosipod/Wled_Auto_ui
Thank you ,

@w00000dy
Copy link
Contributor Author

@dosipod Perfect! By the way, my name on the WLED discord is Woody.

@w00000dy
Copy link
Contributor Author

@blazoncek I noticed a bug in this PR. If you change inlined files like index.js, the script does not recognize the changes. I'm looking for a fix for this and will add it to #3666.

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.

3 participants