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

Display the full path of the defaeult prebuild sandbox #39

Closed
wants to merge 2 commits into from

Conversation

cnadeau
Copy link

@cnadeau cnadeau commented Nov 8, 2020

No description provided.

end

def prebuild_delta_path
@dsl_config[:prebuild_delta_path] || @deprecated_config["prebuild_delta_path"] || "_Prebuild_delta/changes.json"
@dsl_config[:prebuild_delta_path] || @deprecated_config["prebuild_delta_path"] || "#{Dir.pwd}/_Prebuild_delta/changes.json"
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Layout/LineLength: Line is too long. [129/120]

@trinhngocthuyen
Copy link
Contributor

Thanks for the MR. I wonder what lead you to this change. Was there any issue with the current implementation that we need to be aware of?

@cnadeau
Copy link
Author

cnadeau commented Nov 10, 2020

@trinhngocthuyen I was running in fastlane and I lost an hour before I realized that the _Prebuild was ignored because the prebuild command was run in the fastlane folder, not in the root of the project where the Podfile is. I would have seen it immediately if that log has been in place at that time. Nothing to report other than helping anyone getting in the same situation in the future

@trinhngocthuyen
Copy link
Contributor

@cnadeau Thanks for sharing the context.

I would like to express my concern about this change:

  • I understand that fastlane runs its commands in its folder which caused the issue. However, this issue is not specific to the plugin as it can happen to any other use cases. As an example, running normal pod install (without the plugin) also requires the current working dir to be in the project dir (not the fastlane dir). Running CocoaPods installation would also lead to the same issue.
  • Given that, I think prepending Dir.pwd to the prebuild_sandbox_path should not fix the problem. And we need to revise many other places if we do it this way.
  • What we can do in this case is to surface the issue for investigation by printing the prebuild_sandbox_path. In fact, we did print the prebuild sandbox path in https://github.com/grab/cocoapods-binary-cache/blob/master/lib/cocoapods-binary-cache/hooks/pre_install.rb#L57. But it's only visible with the --verbose option.
    --> Perhaps we can change Pod::UI.message to Pod::UI.puts, I may suggest.

Your thoughts?

@ivanrnld ivanrnld closed this in 8724ada Jan 18, 2021
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