Search Unity

IPostprocessBuildWithReport and QA embarrasing answer about a serious bug

Discussion in 'Testing & Automation' started by bdovaz, May 15, 2020.

  1. bdovaz

    bdovaz

    Joined:
    Dec 10, 2011
    Posts:
    1,047
    I reported the following bug:

    https://issuetracker.unity3d.com/is...d-function-is-not-called-when-the-build-fails

    Answer from QA:

    The API:

    https://docs.unity3d.com/ScriptReference/Build.IPostprocessBuildWithReport.html

    This object has the info:

    https://docs.unity3d.com/ScriptReference/Build.Reporting.BuildReport.html

    https://docs.unity3d.com/ScriptReference/Build.Reporting.BuildResult.html

    I think it's embarrassing that answer. I mean, you telling me that depending on X it's called or it's not called. Then call it "IPostprocessSuccessfulBuildWithReport" to make its behavior clear.

    In my case I want to do operations that depend on whether or not it fails and you are saying you can't guarantee that behavior.

    This API is very confusing, there is nothing documented on that subject although in fact it is not something to document but to correct so that it works as it should and always guarantees your call regardless of the step in which it fails.

    You're seriously telling me it's "by design" and it's not going to be corrected?
     
  2. rubengonzalezlodeiro

    rubengonzalezlodeiro

    Joined:
    Apr 23, 2013
    Posts:
    2
    I totally agree with it. This makes the "do something in Preprocess and undo it in Postprocess" completely impossible.
     
    Atomiz2002, Thygrrr and s-sixteen like this.
  3. Ryanc_unity

    Ryanc_unity

    Unity Technologies

    Joined:
    Jul 22, 2015
    Posts:
    332
    Good morning all,
    I'm with the Build Pipeline & Asset Bundle team in charge of this behavior, and also the one that made that decision, not QA. These callbacks were implemented to allow developers to hook into the build pipeline to do work on the build in progress. Such as modify scenes being built with the IProcessSceneWithReport callback, or manipulate the built assemblies with the IPostBuildPlayerScriptDLLs callback.

    When a build fails, we don't continue working on the build as that just doesn't make sense. That work is a waste of time and cycles to developers because the build is unusable, for whatever reason. In fact, failing a build quickly is so important, we get quite a substantial number of bugs around how to make it fail even faster.

    From what I understand based on the Fogbug is that you want to read information from the BuildReport after the build has been complete, success or failure. Have you looked at using this delegate: https://docs.unity3d.com/ScriptReference/BuildPlayerWindow.RegisterBuildPlayerHandler.html This function allows you to hook into the build at the call site, so your code wraps the BuildPipeline.BuildPlayer api, and thus have full access to the returned BuildReport in it's final state.
     
    Last edited: May 15, 2020
  4. liortal

    liortal

    Joined:
    Oct 17, 2012
    Posts:
    3,562
    @Ryanc_unity in case of a failure, is there a BuildReport file stored to disk (under whatever folder it is usually saved, forgot the details now) ?
     
  5. rubengonzalezlodeiro

    rubengonzalezlodeiro

    Joined:
    Apr 23, 2013
    Posts:
    2
    That's not enough. From what I understand from the docs, that callback is called before the build happens, and only allow to change the options for the build, but not to stop it completely which would be the only way to make a custom call to BuildPlayer API and thus hace access to the return value.
    Correct me if I'm wrong, but basically that callback provides another oportunity to do a PreProcess, but not a PostProcess.
     
  6. Ryanc_unity

    Ryanc_unity

    Unity Technologies

    Joined:
    Jul 22, 2015
    Posts:
    332
    Ya, the last build report is stored in the Library folder. I think the file is called lastBuildReport. Have to double check, having issues with my work pc, will follow up in a bit.

    This is not a callback, think of this as an override. If you connect this delegate to your function, but your function doesn't call BuildPipeline.BuildPlayer, or BuildPlayerWindow.DefaultBuildMethods.BuildPlayer, the player will not actually be built. So you have full control at this point to change options for the build, do pre-build setup, call the build api, then post-build cleanup.
     
  7. Ryanc_unity

    Ryanc_unity

    Unity Technologies

    Joined:
    Jul 22, 2015
    Posts:
    332
    Ok, computer data recovered...close call.
    The last build stores it's build report here: "Library/LastBuild.buildreport"
     
  8. jgraves_avr

    jgraves_avr

    Joined:
    Oct 19, 2018
    Posts:
    1
    While this is a complete hack, I've come up with the following solution if you're still looking at implementing something depending on success / failure / cancellation of a build. Hoping the Build Pipeline team at Unity will reconsider the IPostProcessBuildWithReport interface only working when a build successfully completes so that our build automation isn't full of these hacks. The fact that I have to use the IPreprocessBuildWithReport to do something PostBuild is just bad design.

    Code (CSharp):
    1.     void IPreprocessBuildWithReport.OnPreprocessBuild(BuildReport report)
    2.     {
    3.         WaitForBuildCompletion(report);
    4.     }
    5.  
    6.  
    7.     static async void WaitForBuildCompletion(BuildReport report)
    8.     {
    9.         while (report.summary.result == BuildResult.Unknown)
    10.         {
    11.             //some arbitrary about of time meanwhile we wait for the build to complete, can't be bothered to find a better solution.
    12.             await Task.Delay(1000);
    13.         }
    14.  
    15.         switch (report.summary.result)
    16.         {
    17.             case BuildResult.Cancelled:
    18.                 Debug.Log("Build Cancelled");
    19.                 break;
    20.             case BuildResult.Failed:
    21.                 Debug.Log("Build Failed");
    22.                 break;
    23.             case BuildResult.Succeeded:
    24.                 Debug.Log("Build Succeeded");
    25.                 break;
    26.         }
    27.     }
     
    bestknighter, SisusCo, Frolky and 3 others like this.
  9. Xarbrough

    Xarbrough

    Joined:
    Dec 11, 2014
    Posts:
    1,188
    I'd also like to request this as a new feature introduced to the BuildPipeline: A callback that is invoked either on build failure, or always, but with a reliable status of success, cancelled and failed.

    There are so many use cases for having to know that a build failed. Just to name one: I create custom assets OnPreprocessBuild so that they are added to the player, but if this fails, I need to remove them again from the workspace. Works fine on successful build via OnPostprocessBuild, but not if there were any compilation errors (or something else might have gone wrong).

    I don't think wrapping Unity's build methods is the right way to go, because this would limit us to a single client call. But what if I have multiple tools needing such a callback? Sure I do manage my own build process for the build server CI, but developers also use Build and Run for testing, and so I also need to handle this. Obviously, Unity logs "build failed" to the console, so they could also turn this into an event, just as a naive idea...
     
    Atomiz2002 and s-sixteen like this.
  10. SG_roboryantron

    SG_roboryantron

    Joined:
    Apr 2, 2014
    Posts:
    4
    I have a bit of a hack for this that seems to work on Unity 2020.2.
    You can register for the next editor update when building. If the build succeeds, OnPostprocessBuild is called before BuildCheck. This is likely not a very intentional (or possibly even deterministic) pattern, but it may be worth looking in to.

    Code (CSharp):
    1.  
    2.        public void OnPreprocessBuild(BuildReport report)
    3.         {
    4.             // Create Files
    5.             Debug.Log("preprocess");
    6.             EditorApplication.update += BuildCheck;
    7.         }
    8.  
    9.         private void BuildCheck()
    10.         {
    11.             Debug.Log("building " + BuildPipeline.isBuildingPlayer);
    12.             if (!BuildPipeline.isBuildingPlayer)
    13.             {
    14.                 EditorApplication.update -= BuildCheck;
    15.                 OnPostprocessBuild(null);
    16.             }
    17.         }
    18.  
    19.         public void OnPostprocessBuild(BuildReport report)
    20.         {
    21.             // Delete Files (check if they exist first)
    22.             Debug.Log("postprocess");
    23.         }
     
    Skibitsky, Xarbrough and s-sixteen like this.
  11. s-sixteen

    s-sixteen

    Joined:
    Aug 22, 2015
    Posts:
    7
    @Xarbrough summarized the problem really well.

    BuildPlayerWindow.RegisterBuildPlayerHandler
    is not a solution for everybody, since the build process can be launched in many ways...

    What we really need is an interface that gets called no matter what the build result is. It could have the same signature as
    OnPostProcessBuild
    , the supplied BuildReport could be inspected to determine the build status, dropping all work if
    BuildReport.summary.result != BuildResult.Succeeded
    (this can even be suggested in the docs and in the supplied example)

    From an architectural PoV, this would not be a breaking change, all existing scripts would continue to work (as best as they can for now, anyway) and future scripts could leverage this new interface.

    Would this allow package maintainers to write bad code? Well, maybe. But had they decided to do the same as @SG_roboryantron suggested (nice hack, btw!), it would be even worse
     
    Atomiz2002 and Oneiros90 like this.
  12. office_gamelab

    office_gamelab

    Joined:
    Nov 27, 2017
    Posts:
    46
    @ryanc-unity Then how should I know when to remove scripting symbols from the editor?

    I use scripting symbols for automatize building process and set things to create different kind of builds. And then I want to remove the symbols to be able to use the editor correctly after that. Now I need to remove from the editor ~10 symbol after I press Cancel during building, because I realized I didn't set something. And I create 4 builds at once.

    Maybe it doesn't make sense to you to develop delegates for build cancellation, but it makes sense to others for sure.
     
    Last edited: Mar 29, 2021
  13. Tallek

    Tallek

    Joined:
    Apr 21, 2014
    Posts:
    34
    Another vote here for making this callback function after a build failure/cancellation. At a bare minimum, it would be nice if the documentation actually informed us that the callback is not raised in these circumstances. To me, it currently implies the opposite and has cost me some wasted development time today.
     
    v01pe_ and s-sixteen like this.
  14. v01pe_

    v01pe_

    Joined:
    Mar 25, 2015
    Posts:
    71
    I hear you, but a simple delegate to hook onto should not really affect the time that it takes ti fail a build. Developers that are concerned about the CPU cycles can simply ignore the delegate and those who don't care can use it.

    Thanks for the hint. This can be used as a workaround to manually build said functionality. Just be aware, that using this from multiple spots will override previous registrations (there's at least a warning for this case, which saved me some trouble, when realizing the registration was done by an other script already).
    To work around that and the missing cancel delegate, I transformed the suggestion into a simple script, to be found here: BuildHandler.cs
    This example also shows, that there's hardly any overhead, if the `OnBuildCleanup` event isn't subscribed to.
     
    SolidAlloy likes this.
  15. Skibitsky

    Skibitsky

    Joined:
    Mar 22, 2016
    Posts:
    24
    I would also like to have a callback for build failure.

    I do code generation in
    OnPreprocessBuild
    , and remove it in
    OnPostprocessBuild
    . If build fails, all generated code remains in the project and can be accidentally pushed into VCS.
     
  16. _geo__

    _geo__

    Joined:
    Feb 26, 2014
    Posts:
    1,331
    Bump, just to support the request of getting this implementation changed or extended. At least mention it in the docs: https://docs.unity3d.com/ScriptReference/Build.IPostprocessBuildWithReport.html

    BuildReport having an Error and Success result threw me off too. I had to Google just to land here. It's not a good experience. I have sent a feedback comment on the documentation page about it, hope they read it ;-)
     
  17. Ryanc_unity

    Ryanc_unity

    Unity Technologies

    Joined:
    Jul 22, 2015
    Posts:
    332
    Ping: I haven't forgotten about all this good feedback. Very much do appreciate all the great information and details folks have provided. Sadly I don't have anything I can mention yet.
     
    _geo__, uurha and bdovaz like this.
  18. cf-ie

    cf-ie

    Joined:
    Feb 2, 2022
    Posts:
    13
    Another vote to get this in.
     
  19. Eugenio

    Eugenio

    Joined:
    Mar 21, 2013
    Posts:
    197
    It is unbelievable that in September 2023 this is still an issue: OnPostProcessBuild(BuildReport report) is still called when report.summary.result is Uknown and there are, as far as I know, no other way to have a callback when the actual build has been completed.

    Unity version 220.3.9f1.

    Seeing the previous post, I really don't think it is a bug but if it is, please, let me know and I will submit a bug report.
     
  20. _geo__

    _geo__

    Joined:
    Feb 26, 2014
    Posts:
    1,331
    This has worked for me, though it really is a workaround that should not be needed.

    Code (CSharp):
    1. public void OnPreprocessBuild(BuildReport report)
    2. {
    3.     // We have to do it this way because IPostprocessBuildWithReport is not fired if the build fails:
    4.     // see: https://forum.unity.com/threads/ipostprocessbuildwithreport-and-qa-embarrasing-answer-about-a-serious-bug.891055/
    5.     waitForBuildCompletion(report);
    6. }
    7.  
    8. async void waitForBuildCompletion(BuildReport report)
    9. {
    10.     while (BuildPipeline.isBuildingPlayer || report.summary.result == BuildResult.Unknown)
    11.     {
    12.         await Task.Delay(1000);
    13.     }
    14.  
    15.     OnPostprocessBuild(report);
    16. }
    17.  
    18. public void OnPostprocessBuild(BuildReport report)
    19. {
    20.     // Do your stuff after the build.
    21. }
     
    kdchabuk likes this.
  21. Wriggler

    Wriggler

    Joined:
    Jun 7, 2013
    Posts:
    133
    Just jumping in on this one too, as I've recently discovered this inconsistent functionality recently and it lost me a good couple of hours. @Ryanc_unity : I think you're confusing your code requirements with our code requirements. Of course it makes sense that you early out quickly, but on this side of the fence we don't care about that. We just want to know when the build is finished (either success of failure). There's no good reason why you can't have a formalised API which you call from several places. If the problem is that you can't create a build report due to aborting early then make a different API to fetch the report if it is available.

    Your suggestion for us to instead make a BuildHandler might make your code simpler, but it makes all of our code more complex. I've already got a bunch of different ways of running Unity through our CI system, and now I need to integrate yet another way of building via a BuildHandler too. And it's frustrating, as the whole reason I started down the path of OnPreprocessBuild/OnPostprocessBuild was to try and move some of that CI stuff out of Python and into Unity so we can make more consistent builds from within the Editor. But your API design and weird choices add another layer of complexity.

    As everyone above has said: we just want a thing that tells us when the build finished, irrespective of the inner workings and requirements of the internals of the build system that you manage.

    Thanks for listening,

    Ben
     
    HunterBlessing and bdovaz like this.
  22. Atomiz2002

    Atomiz2002

    Joined:
    Feb 2, 2020
    Posts:
    14
    Upvoting this issue.
    I couldn't possibly imagine how would that behaviour be reasonable and I see absolutely no need for me to add to any argument stated above. Do consider fixing this bug, and letting us decide how and whether or not to make use of it. Deciding instead of your users feels quite belittling and takes nothing more than common sense not to do that in cases where it's an obvious issue.

    To finish off my comment with anything but a rant, here is my specific problem with it (which clearly summarizes some of the previously stated issues by the other devs):
    - Your recent versions of Unity have introduced a common bug which appears once every two builds, where some asset would randomly disappear?? while building and cause the build to fail, only to discover the next build goes without a hickup. I want to detect this unreasonable behaviour and re-run the build in case this bug occurs.
    - Why is it called
    OnPostprocessBuildWithReport
    if it would be called only
    OnSuccessfulPostprocessBuildWithReport
    ?
    - It wouldn't make sense to even check the result with
    report.summary.result
    which renders it useless, but for the sake of misleading the developer, it exists there. Why not let it be a
    successfulReport
    instead of a general build
    report
    to clarify it's purpose.
     
    Wriggler likes this.