Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Download symbols in vanilla builds if --with-symbols is used #5412

Merged
merged 2 commits into from
Jan 11, 2021

Conversation

britzl
Copy link
Contributor

@britzl britzl commented Dec 17, 2020

Fixes #5199

@britzl britzl requested a review from JCash December 17, 2020 18:28
variantSuffix = "_release";
break;
case Bob.VARIANT_HEADLESS:
variantSuffix = "_release";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"headless"

int code = connection.getResponseCode();

if (code == 304) {
logInfo("Status %d: Already cached", code);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the output folder is removed between the downloads?
It's not uncommon to clean the build folder for instance (or .internal/cache)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand the scenario you describe. Should we download the vanilla symbols later in the build process you mean?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is, what would happen if you get a 304, but the target file doesn't exist? (e.g. because the build folder was removed between runs)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, got it. Yes, that's a problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we don't really handle that very well when resolving libs either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I think of it I don't believe the server can respond with a 304 since I do not include any request headers indicating ETag or Last-Modified.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I guess you shouldn't handle it here either, since if one would start to send the Etag, this code would fail.

@@ -76,6 +76,7 @@ private void printProgress() {

@Override
public void beginTask(String name, int work) {
System.out.print(name);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug prints?
Won't this spam the console for all resources being built in a project?
If so, it should probably be configurable, and also be done via a logger (where we can format the output)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't beginTask() only used for the main high level build steps such as "Building", "Bundling", "Cleaning" etc?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure. But if it's so, then It might probably look fine.
We really need to look into logging overall, since it's spotty at best, and also uses different logging techniques (which we cannot currently switch on or off)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. We should use Log4j or similar.

int code = connection.getResponseCode();

if (code == 304) {
logInfo("Status %d: Already cached", code);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is, what would happen if you get a 304, but the target file doesn't exist? (e.g. because the build folder was removed between runs)

@britzl britzl merged commit 9a6ad4e into dev Jan 11, 2021
@britzl britzl deleted the issue-5199-download-symbols-in-vanilla-builds branch January 11, 2021 14:09
pull bot pushed a commit to proteanblank/defold that referenced this pull request Jul 28, 2021
…5412)

* Download symbols in vanilla builds if --with-symbols is used

* Review fix. Typo.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When the engine is vanilla the "Generate debug symbols" checkbox in the editor does nothing
2 participants