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

Fix: Adding absolute paths to build.py and gn.py #59798

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

thomasrahimi
Copy link

@thomasrahimi thomasrahimi commented Dec 23, 2024

Changed build.py and gn.py to allow for differing absolute paths. This case may occur when building dart-sdk under Linux

signed-off-by: Thomas Rahimi thomascrrahimi@gmail.com

  • While trying to build dart-sdk for Linux in an operating system, which provides some of the listed dependencies, I experienced issues with the paths included in the file config/build.py and config/gn.py. The current pull requests aims to fix on this issue and merely serves the purpose to ease the packaging efforts of dart under new operating systems.
  • By submitting this pull request I sign the Google Individual Contributor License Agreement (CLA)

Changed build.py and gn.py to allow for differing absolute paths. This case may occurr when building dart-sdk under Linux
signed-off-by: Thomas Rahimi <thomas.rahimi@mailbox.org>
Copy link

google-cla bot commented Dec 23, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@thomasrahimi thomasrahimi changed the title Fix: Fix: Adding absolute paths to build.py and gn.py Dec 23, 2024
Copy link

Thank you for your contribution! This project uses Gerrit for code reviews. Your pull request has automatically been converted into a code review at:

https://dart-review.googlesource.com/c/sdk/+/402282

Please wait for a developer to review your code review at the above link; you can speed up the review if you sign into Gerrit and manually add a reviewer that has recently worked on the relevant code. See CONTRIBUTING.md to learn how to upload changes to Gerrit directly.

Additional commits pushed to this PR will update both the PR and the corresponding Gerrit CL. After the review is complete on the CL, your reviewer will merge the CL (automatically closing this PR).

Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

1 similar comment
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

Added another relative path variable to allow build in non standard environments
signed-off-by: Thomas Rahimi <thomascrrahimi@gmail.com>
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

1 similar comment
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

1 similar comment
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

1 similar comment
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

1 similar comment
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

1 similar comment
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

1 similar comment
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

1 similar comment
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

1 similar comment
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

@mit-mit
Copy link
Member

mit-mit commented Jan 6, 2025

Issue was assigned in https://dart-review.googlesource.com/c/sdk/+/402282

Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

1 similar comment
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

…by gclient. The overall aim is to be able to build dart from source without the necessity for binary files present in the build process.

signed-off-by: Thomas Rahimi <thomascrrahimi@gmail.com>
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

1 similar comment
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

signed-off-by: Thomas Rahimi <thomascrrahimi@gmail.com>
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

1 similar comment
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

…ay, that the need for binary dart packages within the build process can be omitted. The reason to submit this fix is, that Linux distributions requiring the complete build to be based on source code may also not accept to receive binary packages as part of the build process (cf. OpenSUSE project packaging guidelines [https://en.opensuse.org/openSUSE:Packaging_guidelines]).

The current commit introduces a new target OS, which changes the settings of the BUILD.gn used for building dart-sdk.
signed-off-by: Thomas Rahimi <thomascrrahimi@gmail.com>
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

1 similar comment
Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

Copy link

https://dart-review.googlesource.com/c/sdk/+/402282 has been updated with the latest commits from this pull request.

@athomas
Copy link
Member

athomas commented Jan 16, 2025

@thomasrahimi please see the comments on https://dart-review.googlesource.com/c/sdk/+/402282.

@thomasrahimi
Copy link
Author

thomasrahimi commented Jan 17, 2025

@thomasrahimi please see the comments on https://dart-review.googlesource.com/c/sdk/+/402282.

Hello,
thanks for your feedback. I had a look at your comments and got the following replies:
Regarding 171: I agree with your suggestion. I will alter the relevant scripts in such a way, that an additional parameter will be introduced and this will allow to run the build.py under different path settings.
Regarding 222: Is an additional safeguard against path confusion.
I will alter my pull request to match the reply of 171.

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.

3 participants