stata builder bugs

[zhizhongpu]

Opening this issue to report two bugs with the stata builder:

1. whenever a do file contains a name with . in the filename before .do, scons raises an uninformative error
   • demo: 61c6ad4
   • this only affects stata. for example, renaming build.py into build.v2.py is fine
   • expected behavior: scons does not raise an error, or at least raises an informative error
2. when a do file raises an error but all outputs/targets are produced, scons does not raise this error
   • demo: 6ce5c64
   • this only affects stata. for example, having a failed assertion at the end of a python script does induce a scons error
   • expected behavior: scons breaks

My OS is MacOS. Python version 3.12. STATA version 19.5

[zhizhongpu]

@jmshapir normally I'd suggest the first next step to be asking @liaochris or someone else to see if they can replicate the bug, but we've also not been using the stata builder for a while, so I'm looping you in early in case you'd like to weigh in.

[jmshapir]

@zhizhongpu thanks. I'd be more concerned about (2) than about (1) since (2) means no error is raised when an error is likely expected. Happy for us to see if we can fix both when we have some bandwidth for that.

[zhizhongpu]

@zhizhongpu thanks. I'd be more concerned about (2) than about (1) since (2) means no error is raised when an error is likely expected. Happy for us to see if we can fix both when we have some bandwidth for that.

thanks @jmshapir.

@liaochris when the time comes for us to start here, please see if you can replicate bug No. 2. in #149 (comment)

[liaochris]

@zhizhongpu I replicate the error (it's widely known, see here). I proposed a fix in e4f39ae. Let me know what you think.

[zhizhongpu]

thanks @liaochris

in 1952c11 I confirm that the fix works on my local: scons now correctly raises an error when STATA fails and the logs leave a paper trail leading to the assertion error.

I've also tested locally to confirm that scons works as expected when STATA does not raise an error.

the next step seems to me to:

• confirm that the fix works on a Slurm-manged device with STATA 17.0
• decide whether we'd fix bug 1 in stata builder bugs #149 (comment)

fyi @jmshapir

[liaochris]

@zhizhongpu Thanks. Thoughts on parallelizing (I'll look into slurm, you can look into bug 1?)

[liaochris]

Confirming that the solution works on slurm

[liaochris]

@zhizhongpu Following up on whether you have updates on this issue - if we don't want to fix bug 1 from #149 (comment) we can probably open a pull?

[liaochris]

@zhizhongpu Fixed (1) from #149.

The fix disallows periods in .do filenames. Stata batch mode names the log after the do file stem up to the first period — so takelogs.do and takelogs.v2.do would both produce takelogs.log. We now raise a BadExtensionError at build time pointing users to rename instead.