Replies: 6 comments 4 replies
-
Hi @gutley,
Hopefully we can resolve the issue soon! Thanks for asking about this. |
Beta Was this translation helpful? Give feedback.
-
Hi @lmarceau, I've been doing some further digging and am beginning to understand what is happening without yet getting the underlying cause. I've also got a fairly brute force workaround which hasn't failed me yet in my testing. TL;DR In the Glean SDK Generator Build phase, at some point after Evidence I modified the Xcode build phase as follows: rm -rf .venv; bash $SRCROOT/sdk_generator.sh -g MozillaAppServices
date
stat -x $SRCROOT/Client/Generated/Metrics.swift
sleep 30
stat -x $SRCROOT/Client/Generated/Metrics.swift
Here is an example of the output when the build succeeds:
Here everything is as expected. The file has the same dates (ignoring AccessDate) and inode before and after the sleep and they are all <= the time when sdk_generator.sh finished. Now lets look at an example where things went weird:
Here we can see that not only is the file modification date of the Metrics file at the end of the script is after sdk_generator.sh supposedly finished, but we can also see (via the inode) that it is a completely different file to the one that was ready immediately after sdk_generator.sh. This is what leads me to believe that there is something async about sdk_generator.sh and that because it can result in two files being created, that the YAML is actually being done twice. Unfortunately my knowledge of Python isn't going to help debug this any further. Hopefully someone else can see the 'obvious' error in the sdk_generator.sh script that causes this behaviour. For now, in my fork, I am simply going to leave that 30 second sleep in as that ensures the the Metrics file is really complete before continuing on with the compiling. For most builds, Xcode dependency checking will not even call the Glean Script Phase so it doesn't impact overall build times unless the original .yaml file gets updated. |
Beta Was this translation helpful? Give feedback.
-
@lmarceau Good find. That does seem the same problem as ours. I'll test it here tomorrow.
Not sure that is needed in Firefox and will result in all builds being slower as that script takes a few seconds at the best of times. Since we specify input and output files, Xcode 'should' be happy with that. I also saw that bit in the other thread from the Apple Dev about not using Build phases to generate code. Maybe a better solution would be if we could move that generation where its 'meant' to go. |
Beta Was this translation helpful? Give feedback.
-
@lmarceau OK, I've run your new version here without issue so far, so looking good. I did try moving things over to the build rules, but couldn't get the magic incantations to get that working, so for now I suggest we stick with the != indexbuild workaround. I do suggest one small change to you script: if [ $ACTION != "indexbuild" ]; then
rm -rf .venv
bash $SRCROOT/sdk_generator.sh -g MozillaAppServices
echo "\n// Created via $ACTION action" >> "${SRCROOT}/Client/Generated/Metrics.swift"
fi That echo appends a comment with the action type to the generated Metrics.swift file: e.g: .....
enum Device {
/// True if the device support device owner authentication
/// with either biometrics or a passcode.
static let authentication = BooleanMetricType( // generated from device.authentication
category: "device",
name: "authentication",
sendInPings: ["metrics"],
lifetime: .application,
disabled: false
)
}
}
// Created via build action So If this starts failing again, we'll be able to tell what action actually last created the file and if necessary exclude that action alongside indexbuild. (i.e in case Xcode introduce a new incompatable action type in future) I also left the "Based on dependancy analysis" option turned on without issue, so suggest you do the same when you commit your changes. |
Beta Was this translation helpful? Give feedback.
-
It sounds like the above fix works, but what about the two suggestions that the R.swift team got from Apple?
mac-cain13/R.swift#719 (comment) Is that something we want to do? If this script is really running in the wrong place then maybe those suggestions are better? |
Beta Was this translation helpful? Give feedback.
-
@gutley Update on this. The Glean team went forward to merge the indexbuild fix in the So next time A-S is upgraded, we'll get that fix too. Long term plan is still to improve the way we generate builds as @st3fan suggested thought. |
Beta Was this translation helpful? Give feedback.
-
A few times a day, I fail to build the app with the following error:
Once this happens, the only way to solve it is to clean the project and try again.
Its getting a bit irksome.
Beta Was this translation helpful? Give feedback.
All reactions