Skip to content

glog: disable broken test#371915

Merged
vcunat merged 1 commit intoNixOS:stagingfrom
FliegendeWurst:glog-pie
Feb 8, 2025
Merged

glog: disable broken test#371915
vcunat merged 1 commit intoNixOS:stagingfrom
FliegendeWurst:glog-pie

Conversation

@FliegendeWurst
Copy link
Member

@FliegendeWurst FliegendeWurst commented Jan 7, 2025

Full log: https://paste.fliegendewurst.eu/YLB4qa

ref. #205031

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Fits CONTRIBUTING.md.

@emilazy
Copy link
Member

emilazy commented Jan 7, 2025

I think just unconditionally disabling the test would be better (as long as downstream packages do in fact work).

@github-actions github-actions bot added 10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. labels Jan 7, 2025
@nix-owners nix-owners bot requested review from nh2 and r-burns January 7, 2025 21:05
@FliegendeWurst
Copy link
Member Author

It does look like it fails in library code: https://github.com/google/glog/blob/v0.7.1/src/logging.cc#L2236-L2340
Since the log stops after [ RUN ] EmailLogging.ValidAddress, which just goes straight to that function.

I am a bit confused no one else noticed the failure so far, since PIE is the default elsewhere..

@FliegendeWurst FliegendeWurst changed the title glog: disable PIE hardening glog: disable broken test Jan 10, 2025
@FliegendeWurst
Copy link
Member Author

Now disabled unconditionally, after I observed a failure with PIE disabled

@FliegendeWurst FliegendeWurst force-pushed the glog-pie branch 2 times, most recently from 8836286 to d09ad18 Compare January 10, 2025 22:25
@FliegendeWurst FliegendeWurst changed the title glog: disable broken test glog: disable broken test, fix pkg-config generation Jan 10, 2025
@FliegendeWurst FliegendeWurst changed the base branch from staging to staging-next January 10, 2025 22:31
@FliegendeWurst
Copy link
Member Author

FliegendeWurst commented Jan 10, 2025

Moved to staging-next, with an additional fix for missing libglog.pc. (Broke many downstream packages previously.)

@emilazy
Copy link
Member

emilazy commented Jan 10, 2025

Broke many downstream packages previously.

Has this been seen in the current staging-next cycle? AFAICT the change in the original PR was purely prophylactic, and I suspect that the .pc might not work due to the same changes that have required CMake fixes in the Facebook packages. If we don’t have evidence of packages needing this I’d prefer to follow upstream and leave it out.

@FliegendeWurst
Copy link
Member Author

FliegendeWurst commented Jan 11, 2025

I have evidence of at least one package that no longer works: the lomiri download manager.
https://paste.fliegendewurst.eu/4k9VYa.log

@FliegendeWurst
Copy link
Member Author

By "previously" I just meant the state before this PR. The .pc file seems to work for the mentioned lomiri package.

@emilazy
Copy link
Member

emilazy commented Jan 13, 2025

Does it actually build successfully with this change, given the #define that’s now required?

@FliegendeWurst
Copy link
Member Author

glog builds, but the lomiri-download-manager does not. I'll drop the pkg-config changes then.

@FliegendeWurst FliegendeWurst changed the title glog: disable broken test, fix pkg-config generation glog: disable broken test Jan 13, 2025
@emilazy
Copy link
Member

emilazy commented Jan 13, 2025

Looks like you dropped the wrong commit.

@emilazy
Copy link
Member

emilazy commented Jan 13, 2025

Looks good now, but will presumably have to go to the next staging cycle given the amount of rebuilds.

@FliegendeWurst FliegendeWurst changed the base branch from staging-next to staging January 13, 2025 21:54
@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jan 14, 2025
@FliegendeWurst
Copy link
Member Author

FliegendeWurst commented Jan 15, 2025

For reference, here are the broken packages I found so far.

@emilazy
Copy link
Member

emilazy commented Jan 15, 2025

Yeah. folly and friends were broken too. You can look at #371610 for how to patch downstream CMake files to hold glog correctly.

@OPNA2608
Copy link
Contributor

For reference, here are the broken packages I found so far.

[Mir & Lomiri things]

Working on fixing these upstream & in Nixpkgs, feel free to ignore them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants