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

Use latest release from beginning #16

Closed
wants to merge 1 commit into from

Conversation

klep
Copy link

@klep klep commented Oct 31, 2021

The branch master no longer exists in the repo -- it has been renamed main. However, I think we're better off using the latest tag, since that's what's used later when we do the final build.

@@ -123,7 +123,7 @@ export PATH="${PWD}/depot_tools:${PATH}"
cd skia

# get proper skia's branch to compile
SKIA_BRANCH=$(curl "https://raw.githubusercontent.com/aseprite/aseprite/master/INSTALL.md" | grep "aseprite-m[0-9][0-9]" | sed -n '1p' | perl -n -e '/(aseprite-m\d\d)/ && print $1')
SKIA_BRANCH=$(curl "https://raw.githubusercontent.com/aseprite/aseprite/main/INSTALL.md" | grep "aseprite-m[0-9][0-9]" | sed -n '1p' | perl -n -e '/(aseprite-m\d\d)/ && print $1')
Copy link
Author

Choose a reason for hiding this comment

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

This is arguably still wrong, since it's not using the tag. I think it's probably okay for now.

Copy link
Owner

@haxpor haxpor Nov 1, 2021

Choose a reason for hiding this comment

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

Thanks. This one is good. aseprite really changes the structure of branches included separation of release and beta version.

Edit:
Yeah, I kinda agree, it's ideal to base on specific tag to parse the content from.

@@ -3,7 +3,7 @@
# Script to automate building latest release of Aseprite (it can be release or beta build)
# This is for macOS build version.

POSTFIXPATH_SDKROOT=Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk
POSTFIXPATH_SDKROOT=Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk
Copy link
Author

Choose a reason for hiding this comment

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

It should be fine to use the current installed SDK, rather than requiring an old one...

Copy link
Owner

Choose a reason for hiding this comment

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

I'm skeptical about this as it relates to #11 , I think we should not touch this for now.

@haxpor
Copy link
Owner

haxpor commented Nov 1, 2021

@klep I did check and upstream aseprite changed git branch structure to hold normal release (stable version), and beta version into separate branch. They are respectively main, and beta branches now.

I believe we would need to modify aseprite.sh to allow users to explicitly choose whether they want stable or beta version to build. Overall git related commands in aseprite.sh will still be fine but especially we will checkout a proper branch (either main or beta) then existing command of

git checkout `git describe --tags` 

will return a correct result for the latest recent tag corresponding to which branch such tag coming from. Now it return unexpected tag.


Do you want to work on this yourself or I can do it from my side? regarding to the following changes

  1. Accept command line argument of --beta, but by default we will build against normal stable release via main branch.
  2. Change accordingly to git related command to checkout a proper branch as we knew user's intent from 1.
  3. Cleanup misc URL that still bases on master, or adjust accordingly.

Feel free to let me know! Thanks!

@haxpor haxpor added enhancement upstream-changed Upstream aseprite has changed, we need to update ours. labels Nov 1, 2021
@klep
Copy link
Author

klep commented Nov 3, 2021

Happy to handle it either way. I suspect this isn't a full time focus for either of us, so maybe just whomever gets to it first? I'll have some in a few weeks, but won't be offended if you make the fix without me :)

@haxpor
Copy link
Owner

haxpor commented Nov 3, 2021

Thanks @klep . I asked to respect your initiation, and continued effort in making changes. I'll do it but will mention and link in your finding/effort in the commit message. Thank you again :)

I close this issue for now. The proper fix will be worked on later.

@haxpor
Copy link
Owner

haxpor commented Nov 3, 2021

This can be tracked at #17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement upstream-changed Upstream aseprite has changed, we need to update ours.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants