Skip to content

Exclude Windows-specific CCTZ source files in podspec #2138

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.

Sign up for GitHub

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

Open
aslam9691 wants to merge 1 commit into
base: master
Choose a base branch
from
+8 −1
Open

Exclude Windows-specific CCTZ source files in podspec #2138

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion absl/abseil.podspec.gen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
s.tvos.deployment_target = '12.0'
s.watchos.deployment_target = '4.0'
s.visionos.deployment_target = '1.0'
s.exclude_files = [ 'absl/time/internal/cctz/src/*_win.cc' ]
s.subspec 'xcprivacy' do |ss|
ss.resource_bundles = {
ss.module_name => 'PrivacyInfo.xcprivacy',
Expand Down Expand Up @@ -189,12 +190,18 @@ def write_podspec_rule(f, rule, depth):
# Since CocoaPods treats header_files a bit differently from bazel,
# this won't generate a header_files field so that all source_files
# are considered as header files.
srcs = sorted(set(rule.hdrs + rule.textual_hdrs + rule.srcs))
srcs = [
s
for s in sorted(set(rule.hdrs + rule.textual_hdrs + rule.srcs))
if not s.endswith("_win.cc")
]
write_indented_list(
f, "{indent}{var}.source_files = ".format(indent=indent, var=spec_var),
srcs)
# Writes dependencies of this rule.
for dep in sorted(rule.deps):
if not dep.startswith("//absl/"):

@derekmauro derekmauro Aug 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change looks unrelated to the problem being fixed. Can you explain it to me?

@aslam9691 aslam9691 Aug 27, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

If a dependency with a different prefix was passed to get_spec_name, it would fail the assertion and throw an AssertionError. for the safety added this check.

@derekmauro derekmauro Aug 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is that actually happening? Is that preferred to a silent error?

@aslam9691 aslam9691 Aug 28, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It's not happening right now, I just added it defensively for edge cases. I originally thought skipping it was safer than crashing the script, but if you prefer an explicit assertion failure to catch bad inputs early, I am happy to remove this check.

@derekmauro derekmauro Aug 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In general, "defensive programming" is for defending against things outside of your control. Here the assertion is a good thing. We are checking our own invariants. An error would be our fault, and it is best to stop immediately with a loud error.

It also also a good rule not to make unrelated changes like this one.

Please revert this part of the change.

@aslam9691 aslam9691 Aug 31, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

My Bad

after removing podspec generation is failing with the following error.

File "/Users/aslamansari/Desktop/g-workspace/abseil-cpp/absl/abseil.podspec.gen.py", line 250, in
main()
File "/Users/aslamansari/Desktop/g-workspace/abseil-cpp/absl/abseil.podspec.gen.py", line 246, in main
generate(args)
File "/Users/aslamansari/Desktop/g-workspace/abseil-cpp/absl/abseil.podspec.gen.py", line 225, in generate
write_podspec(f, rules, vars(args))
File "/Users/aslamansari/Desktop/g-workspace/abseil-cpp/absl/abseil.podspec.gen.py", line 154, in write_podspec
write_podspec_map(f, rule_dir, 0)
File "/Users/aslamansari/Desktop/g-workspace/abseil-cpp/absl/abseil.podspec.gen.py", line 179, in write_podspec_map
write_podspec_map(f, value, depth + 1)
File "/Users/aslamansari/Desktop/g-workspace/abseil-cpp/absl/abseil.podspec.gen.py", line 181, in write_podspec_map
write_podspec_rule(f, value, depth + 1)
File "/Users/aslamansari/Desktop/g-workspace/abseil-cpp/absl/abseil.podspec.gen.py", line 203, in write_podspec_rule
name = get_spec_name(dep.replace(":", "/"))
File "/Users/aslamansari/Desktop/g-workspace/abseil-cpp/absl/abseil.podspec.gen.py", line 140, in get_spec_name
assert label.startswith("//absl/"), "{} doesn't start with //absl/".format(
AssertionError: @do_not_use_for_gloop_visibility_only//gloop/base/fprint doesn't start with //absl/

@derekmauro derekmauro Aug 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is why we don't suppress errors like this. That dependency is a mistake and should be removed.

I also just noticed that you are a Googler. Googler's are required to send changes using the internal code process. You should have noticed this in the pull request template. https://github.com/abseil/abseil-cpp/blob/master/.github/PULL_REQUEST_TEMPLATE.md

The internal change to remove the dependency is cl/973833051. After that is submitted and you remove the check I will merge this, but in the future, please use the internal code review process.

continue
name = get_spec_name(dep.replace(":", "/"))
f.write("{indent}{var}.dependency '{dep}'\n".format(
indent=indent, var=spec_var, dep=name))
Expand Down