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:
    894
    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?
     
    Skibitsky, VolodymyrBS and v01pe_ like this.
  2. EthDra

    EthDra

    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.
     
    s-sixteen likes this.
  3. Ryanc_unity

    Ryanc_unity

    Unity Technologies

    Joined:
    Jul 22, 2015
    Posts:
    324
    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
    jonasj_unity and unity_bill like this.
  4. liortal

    liortal

    Joined:
    Oct 17, 2012
    Posts:
    3,506
    @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. EthDra

    EthDra

    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:
    324
    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:
    324
    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.     }
     
    SisusCo, Frolky, io-games and 2 others like this.
  9. Xarbrough

    Xarbrough

    Joined:
    Dec 11, 2014
    Posts:
    1,089
    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...
     
    s-sixteen likes 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:
    4
    @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
     
  12. office_gamelab

    office_gamelab

    Joined:
    Nov 27, 2017
    Posts:
    35
    @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:
    31
    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:
    65
    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.
     
  15. Skibitsky

    Skibitsky

    Joined:
    Mar 22, 2016
    Posts:
    11
    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.
     
unityunity