[SKIP-HCK-CI] [build] Extend ANSI colour palette to stdout #1284
Conversation
benyamin-codez
force-pushed
the
build-ex-colours
branch
from
4f18af1 to
1078100
Compare
|
Rebase done... |
|
Hope you're well. Did you have anything further for me to consider wrt this one? Best regards, |
kostyanf14
changed the title
|
rerun tests |
buildAll.bat
Outdated
| call schtasks /create /tn build_log_cleanup /tr "%comspec% /c %~dp0build\clean_build_log.bat" /sc ONCE /sd 01/01/1910 /st 00:00 1> nul 2>&1 | ||
| call schtasks /run /tn build_log_cleanup 1> nul 2>&1 |
There was a problem hiding this comment.
I don't like the idea of triggering schtasks. This will work when the building is running on a clean Windows PC but what about docker or Windows containers?
I think we don't need to cleanup files at all.
There was a problem hiding this comment.
Ok, so we will run with the SYSTEM user. This hides the clean_build_log.bat so it will execute without GUI, this should be sufficient for most Docker and container configurations. It also avoids the need to instead use powershell to achieve the same outcome, as this requires the XML configuration to be properly escaped for it to work as expected.
It seems prudent to give a bit more time (+2s = 5s) for handles to close in the powershell context and remove some for the visual check for debugging purposes (-3s = 2s).
NOTE: Some containers might need to be started with the -enableTaskScheduler parameter, but most setups should just work.
I think we don't need to cleanup files at all.
It is very ugly otherwise, like morning after too many 🍻 ... 8^D Per:
�[40;92mBuild for Q35 succeeded.�[�[�[�[0m
�[40;96mProcessing DVL files to create Windows 10, version 1607, WIN10_RS1 COMPAT version...�[�[�[�[0m
�[40;93mWARNING : No DVL files were found.�[�[�[�[0m
�[40;92mBUILD COMPLETED SUCCESSFULLY.�[�[�[�[0m
Vs:
Build for Q35 succeeded.
Processing DVL files to create Windows 10, version 1607, WIN10_RS1 COMPAT version...
WARNING : No DVL files were found.
BUILD COMPLETED SUCCESSFULLY.
There was a problem hiding this comment.
Any further thoughts on this?
There was a problem hiding this comment.
Let's remove this.
- You specified the
build_log.txtfile which is definitely not part of the build. This file can be generated by Jenkins in our case. - Jenkins doesn't know anything about schtasks, so it can start collecting artifacts after
buildAll.batexits.
You should create build/clean_log.bat which receives the log file name for cleanup and cleanup of this file. If someone writes a log to a file, he should also run clean_log.bat for the file that he created.
There was a problem hiding this comment.
Thanks for the further feedback, Kostiantyn.
Re:
(1) Yes, I only catered for your Jenkins-based automation. My thoughts were that others can mimic the naming; &
(2) I previously discussed how the clean should complete before signing is finished. Alternatively, you could issue a sleeper to be sure, e.g.: powershell Start-Sleep -Seconds 7 after building or signing and before the artefact collection occurs.
I can certainly make build\clean_log.bat accept a parameter for filename so it can act in a manual and universal manner. Are you sure you don't want this automated? Is schtasks too sneaky? Are you considering that you will just call build\clean_log.bat build_log.txt before artefact collection (via Jenkins)?
There was a problem hiding this comment.
Let's start with a fully stable variant with the manual solution and then think about an automated variant.
There was a problem hiding this comment.
🤔
... 💡
... ... 👍
There was a problem hiding this comment.
I've introduced a new environment variable AUTOCLRCLEAN_VIRTIO_WIN_BLD_LOG, which corresponds to a log file you want cleaned of colour artefacts. If the variable is defined and the file exists then the clean operation is scheduled.
Also updated build\clean_build_log.bat with more comments, debug prints and the filename parameter, plus added check for SYSTEM user where we now skip flashing the stdout cleaning log output for 5 seconds (for debug purposes). Also made sure that AUTOCLRCLEAN_VIRTIO_WIN_BLD_LOG is both relative and absolute path safe and also file name (white)space safe.
So, this means that unless you pop AUTOCLRCLEAN_VIRTIO_WIN_BLD_LOG with a valid file (path and) name and create that file by some external mechanism, e.g. via Jenkins, then the task will not be created or run (the current behaviour).
I also checked manual operation, per, e.g. build\clean_build_log.bat .\build_log.txt
I trust that is acceptable. If so, enjoy..!!
|
Many thanks for your feedback. Best regards, |
benyamin-codez
force-pushed
the
build-ex-colours
branch
from
2cfd9ef to
ac075a4
Compare
|
Apologies for prior lack of rebase... 8^d |
1. Extended ANSI colour palette to stdout 2. Introduce log functions (credit to @kostyanf14) with colour output 3. Update echo prints to use log functions 4. Introduces environment variable AUTOCLRCLEAN_VIRTIO_WIN_BLD_LOG, which corresponds to a log file you want cleaned of colour artefacts 5. Includes self-deleting scheduled task to clean log file per (4) of colour palette artefacts. Runs in SYSTEM user context for use with Docker and container images. Only runs if the variable AUTOCLRCLEAN_VIRTIO_WIN_BLD_LOG is defined and the file exists. Requires external mechanism to create log file from stdout. Split from PR virtio-win#1212. Signed-off-by: benyamin-codez <[email protected]>
benyamin-codez
force-pushed
the
build-ex-colours
branch
from
ac075a4 to
ece80c2
Compare
1. Extended ANSI colour palette tostdout2. Update
echoprints to use ANSI colour palette3. Includes self-deleting scheduled task to clean
build_log.txtof colour palette artefactsstdoutAUTOCLRCLEAN_VIRTIO_WIN_BLD_LOG, which corresponds to a log file you want cleaned of colour artefactsAUTOCLRCLEAN_VIRTIO_WIN_BLD_LOGis defined and the file exists. Requires external mechanism to create log file fromstdout.Split from PR #1212.