Is there a way to see CircleCI results without giving them access to my entire GitHub account?

Codecov Report

Merging #494 (6cfa9d4) into master (41b59d8) will decrease coverage by 0.00%.
The diff coverage is 92.85%.

@@            Coverage Diff             @@
##           master     #494      +/-   ##
==========================================
- Coverage   79.50%   79.49%   -0.01%     
==========================================
  Files         490      490              
  Lines       29436    29439       +3     
==========================================
+ Hits        23403    23404       +1     
- Misses       6033     6035       +2     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41b59d8...6cfa9d4. Read the comment docs.

Is there a way to see CircleCI results without giving them access to my entire GitHub account?

You should only need to log in via GitHub. There's one a bit weird "first time use" behavior -- if you log in, it doesn't redirect you to the actual build log afterwards, but jumps to some "add watched repositories" page, which kinda looks like it wants access to your everything, yeah. Opening the link again when you're already logged in works.

(Unfortunately Travis CI commited suicide and doesn't bother answering my e-mails so I had to switch providers... but this login requirement is the only major downside of CircleCI so far, another is buggy console color rendering but that's a minor wart compared to what I had to regularly go through with Travis.)

The failed build was just some OOM error, and it works after a restart. I get 36 cores but anywhere between 8 and 32 GB RAM for each build, and sometimes the ultra-parallel jobs won't fit.

Exposing a parameter called InternalFlags feels a bit daring

Yeah 😅 Because this API needs to be public and documented, it should be something else, and without the internal flags that are really not meant to be touched by users :) Give me some time to come up with acceptable naming. Apart from that, the change looks good in general.

You should only need to log in via GitHub

Nope AFAIK you actually need to give write access to every public repo you own just to log in:

By logging in, you are agreeing to our SaaS Agreement and Privacy Policy.
We ask for read/write access to make your experience seamless on CircleCI.
If you’re not ready to give us access to private projects, choose public repos.

Nevermind that I don't want to use it on my repos, I just wanted to view a build output on another user's repo... Anyway, I did that and revoked it immediately afterwards, no problem.

Yeah 😅 Because this API needs to be public and documented, it should be something else, and without the internal flags that are really not meant to be touched by users :)

I actually thought it would be nice to be able to manipulate the logging flags. What I'm doing at the moment to be able to switch off logging for one context is:

// Hide Magnum context initialization for this context
std::vector<const char*> argv{ "dummy",
    "--magnum-log", "quiet"
};
int argc = argv.size();

Platform::GLContext context(argc, argv.data());

and I thought if I could directly un-set the DisplayInitializationLog flag that would be somehow cleaner:

// A silent context without framebuffer.
Platform::GLContext context(GL::Context::InternalFlag::NoFramebuffer);

So in my opinion renaming InternalFlag to CreationFlag or something like that (and documenting it properly) could be an option.

I'm not ignoring you, just having a big pile of things to get through 😅

I actually thought it would be nice to be able to manipulate the logging flags

Yep, that's one of the very common pain points. I realized the remaining --magnum-blah options could make sense to be exposed as well, especially the workarounds, gpu validation and extension disabling, and an API similar to application's Configuration / GLConfiguration might work (and in case of the applications the GLConfiguration would then derive from this to share the options).

So that expands the scope a bit. I'll try to implement and merge this until the end of the week, hope it doesn't stall you too much.

Additionally, while working on something unrelated, I realized there's another potential race here:

Will fix that as well, by just resetting the viewport to zero instead.

So that expands the scope a bit. I'll try to implement and merge this until the end of the week, hope it doesn't stall you too much.

Yes, using a Configuration-style setup sounds good! No worries, I'm fine working with my hacky patch for now :)

Oookay, this is one of the "fixing the lightbulb" moments, because...

• Adding a Context::Configuration means that Platform::*Application::Configuration classes will subclass it, which means they need to pull in <Magnum/GL/Context.h>
  • But that header is rather huge, including <vector> and <string> and I don't want to drag all that nonsense along with every Application, so I proceeded in porting the public API away from these and to Array[View] and StringView
    • Which means I had to implement a bunch of missing APIs such as StringView::contains()
      • And various whitespace trimming functions that used to be only on a std::string before
        • But after all it was a great thing, as I got rid of one std::sort() as well and just that reduced compile times by 5% and binary size by 10 kB, preprocessed size of the Context.h header to half; and the amount of allocations done on startup from about 9253 to 18
          • Which can be further reduced to maybe 2 or 3 using an ArrayTuple
            • And with std::strings going away there's a bunch of other low hanging fruits to take care of, like avoiding all the string copies when populating a GL::Shader because the inputs are mostly just global string literals...

Nevertheless, I stopped the recursion at level 5 for now, I'm done with the base work on the Configuration, so the next step should finally be introducing the new flag 😅

Wow, I feel honored to have kicked off that chain ;) Let me know if I can help in any way.

Okay, finally landed the last bit -- 8aceb25. It's still in the next branch and not master as I need to be sure that it does the right thing first :) Can you check the commit and tell me if it's working how you expect (or point me to parts that I forgot to handle)?

Thank you!

Sorry for the lag ;) It seems to work just fine (tested Windowless | QuietLog), at least it fixes the startup crash I get when running unmodified master 👍

I can test my full setup with streaming data tomorrow when I'm back in the lab, but I expect no issues there.

... actually, there is something missing in GL::Context::tryCreate():

     /* GPU validation is enabled if either enables it */
    [...]

    /* Same for windowless contexts */
    if(configuration.flags() & Configuration::Flag::Windowless)
        _configurationFlags |= Configuration::Flag::Windowless;

Some more info on my usage of the API, in case you need it (or I'm using something wrong):

Main thread:

        worker.glContext = Platform::WindowlessGlxContext{
            Platform::WindowlessGlxContext::Configuration{}
                .setSharedContext(glxContext)
        };

Worker thread:

    // Init our own GL context
    worker.glContext.makeCurrent();
    Platform::GLContext context(GL::Context::Configuration{}.addFlags(
        GL::Context::Configuration::Flag::Windowless | GL::Context::Configuration::Flag::QuietLog
    ));

If I add the snippet above, everything works fine.

That omission caused races on GL::Framebuffer deletion, right? Other than that I can't see a place where it would matter.

Fixed in abd07b6 (next branch again, force-pushed), can you check again? Thanks 🙏

That omission caused races on GL::Framebuffer deletion, right? Other than that I can't see a place where it would matter.

No, actually you check _configurationFlags for the Windowless flag, not configuration. So somehow it needs to be carried over to _configurationFlags ;)

I'll test again.

Works fine 👍

Ah, haha, stupid me. The fix uncovered another stupid bug in my change that broke ES2 builds, fixed that now as well. When the CIs get green, I'll merge to master.

Okay, the Windowless flag is finally in master as 46df57f.