Add features file describing new available flags

The only available method to check for features at the moment is through
version checks. This is errorprone and doesn't work well for OSS
toolchains or locally built compilers.
features.json is intended to communicate to build systems that a new
flag is available, in order to assist with a transitional period where
not all supported toolchains may have a particular flag. It is not
intended to be a comprehensive report of all flags available.
Note that the names are intended to be features, so while they may match
up to the corresponding flag name, this isn't necessarily the case.

@nkcsgexi this is an approach that avoids the complexity of needing to run the just-built compiler.
It's not a comprehensive approach though, it is intended primarily to assist in a transitional period for adopting newly added flags, not to provide a complete overview of all the available options.

@swift-ci please smoke test

@akyrtzi can we just run swift-front -emit-supported-features at compile time to generate the comprehensive list? Having the list pre-generated is a nice to have.

Yet another option would be to just run the preprocessor on a JSON file including Options.inc (like the -emit-supported-features option does, but at compile time). Or is that what you meant @nkcsgexi?

Yeah, in general I'm not a big fan of maintaining this file manually. We could potentially generate this source using the preprocessor. A better option is to change the existing .td file to a python representation and use gyb to generate C++ source and feature.json at compile time like syntax nodes.

Yeah, in general I'm not a big fan of maintaining this file manually.

There's not much maintenance involved given this is for transition of new flags only. Once you add a new flag, intended to be used by a build system, you add a new entry. After about a year we can choose to prune out old entries, assuming all supported toolchains have the flag.
Not all frontend flags we add are intended to be used a build system, having a comprehensive list is nice but I don't see a critical use case and there is inherent complexity to set up.
At a minimum we could start with the manual list of new flags for now, since it is dead-simple, and make it comprehensive later.

I’m not entirely confident that every new commit of adding a compiler flag will be accompanied with an update of this json file. How would we ensure people remember doing that? Also, we need to validate this is a valid json file, ideally not manually.

I’m not entirely confident that every new commit of adding a compiler flag will be accompanied with an update of this json file. How would we ensure people remember doing that?

It is not necessary that every new commit of adding a compiler flag is added to this file. It's only necessary if we intend for the build system to use it, and the fact the build system needs to check for a "feature" name being present in the file before using the new flag is what will force and ensure its presence in the file, even if the initial swift commit happens to forget to update the file.

@swift-ci please build toolchain

Linux Toolchain (Ubuntu 16.04)
Download Toolchain
Git Sha - d57d9fd7268fa4a3493e0bccd07b75754b196782
Install command
tar zxf  swift-PR-34697-497-ubuntu16.04.tar.gz
More info

macOS Toolchain
Download Toolchain
Git Sha - d57d9fd7268fa4a3493e0bccd07b75754b196782
Install command
tar -zxf  swift-PR-34697-772-osx.tar.gz --directory ~/

Linux:
usr/share/swift/features.json
OSX:
Library/Developer/Toolchains/swift-PR-34697-772.xctoolchain/usr/share/swift/features.json
Looks good!


Move Discord bot to a separate server

Hey @bonustrack, for me to understand what should be done :pray:
Couple of questions:

The goal is to move entire logic from discord.ts to a new repo snapshot-discord? (I may need to ask you to create it on GH, not sure if I have the permission to do so)
Should the new repo use the same DB as here or should it have its own db?
Finally this should call the new repo instead of the discord client within snapshot-webhook ?

Any other changes needed here? :thinking:
cc @ChaituVR for tomorrow :pray:

Created a new repo for this https://github.com/snapshot-labs/discord-bot @bonustrack Feel free to rename


Create and publish new java release v0.0.1

Just to get a release started (I understand the spec is not finalized), but we need something to work with for now
note: java release process requires vN.N.N (it's checked in the action). We should change that if we want to do a *.1a1 like python, but that's up to the repo owners.
A repo admin must (maybe @znewman01)

[x] push a tag release/java/v0.0.1

Someone with sonatype[dev.sigstore] publish permissions must (probably @loosebazooka)

[ ] release to maven central

Detailed instructions at https://github.com/sigstore/protobuf-specs/blob/main/java/README.md

https://github.com/sigstore/protobuf-specs/releases/tag/release%2Fjava%2Fv0.0.1

hrmm... something weird happened with the release. like no "release" is present in the releases dir... did someone delete it?

Are you looking for the draft release it created? https://github.com/sigstore/protobuf-specs/releases/tag/untagged-f5746bbc236b7a565d42

(we already figured this out on slack), but it turns out I can't see draft releases because I'm not a project member

well in an insane twist, it looks like the checksums stuff is unnecessary for the release path we take. I'll go clean that up. But the release is otherwise good to go.


Highlighted element is not scrolled into the viewport even if scrollMatches set to true/false

Attach (recommended) or Link to PDF file here:
Configuration:

Web browser and its version: Chrome
Operating system and its version: Macos
PDF.js version: 2.5.207
Is a browser extension: No

Steps to reproduce the problem:

execute a find command and check for the hightlighted element
if the highlighted text is at end of the page or not in viewport the scrollintoview is not behaving properly.

What is the expected behavior? (add screenshot)
highlighted element should be scrolled into the viewport after a match is found.
What went wrong? (add screenshot)
After executing find command, highlighted element is not scrolled into the viewport even if scrollMatches set to true/false,
as of now it only scrolls to the top of the page.
this.pdfFindController.executeCommand("find", {
        query: this.text,
        caseSensitive: false,
        phraseSearch: true,
        highlightAll: false,
        scrollMatches: true,
      });

Link to a viewer (if hosted on a site other than mozilla.github.io/pdf.js or as Firefox/Chrome extension):

Please see https://github.com/mozilla/pdf.js/blob/master/.github/CONTRIBUTING.md (emphasis mine):

If you are developing a custom solution, first check the examples at https://github.com/mozilla/pdf.js#learning and search existing issues. If this does not help, please prepare a short well-documented example that demonstrates the problem and make it accessible online on your website, JS Bin, GitHub, etc. before opening a new issue or contacting us in the Matrix room -- keep in mind that just code snippets won't help us troubleshoot the problem.

Please see https://github.com/mozilla/pdf.js/blob/master/.github/CONTRIBUTING.md (emphasis mine):

If you are developing a custom solution, first check the examples at https://github.com/mozilla/pdf.js#learning and search existing issues. If this does not help, please prepare a short well-documented example that demonstrates the problem and make it accessible online on your website, JS Bin, GitHub, etc. before opening a new issue or contacting us in the Matrix room -- keep in mind that just code snippets won't help us troubleshoot the problem.

@Snuffleupagus if scrollMatches not a valid option, what is the configuration to bring the highlighted text to the viewport?

@Snuffleupagus if scrollMatches not a valid option, what is the configuration to bring the highlighted text to the viewport?

what is the configuration to bring the highlighted text to the viewport?

That happens by default, and there's no way to turn if off, which you should've been able to reproduce yourself in the demo-viewer linked above.
Unless you provide a runnable example, as asked for, which shows the problem, this issue will be closed as INVALID.

what is the configuration to bring the highlighted text to the viewport?

That happens by default, and there's no way to turn if off, which you should've been able to reproduce yourself in the demo-viewer linked above.
Unless you provide a runnable example, as asked for, which shows the problem, this issue will be closed as INVALID.


Refactor code to handle dropdown widgets in organization settings.

First commit is to use a single function to set dropdown widget variables.
Second commit is to extract a function for setting up the dropdown widgets.

Self-review checklist

[ ] Self-reviewed the changes for clarity and maintainability
(variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

[ ] Explains differences from previous plans (e.g., issue description).
[ ] Highlights technical choices and bugs encountered.
[ ] Calls out remaining decisions and concerns.
[ ] Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

[ ] Each commit is a coherent idea.
[ ] Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

[ ] Visual appearance of the changes.
[ ] Responsiveness and internationalization.
[ ] Strings and tooltips.
[ ] End-to-end functionality of buttons, interactions and flows.
[ ] Corner cases, error conditions, and easily imagined bugs.

Noticed a styling bug from the last PR while reviewing this:

Seems to work and feels like a great progress checkpoint, so merged, thanks @sahil839! I think the above can be addressed as follow-ups.


How to customize the storage path?

As mentioned in the title, I want to set the location where the bookmark information is stored, instead of the default .local/share/nvim/bookmarks

As mentioned in the title, I want to set the location where the bookmark information is stored, instead of the default .local/share/nvim/bookmarks

Well, I will add a configuration option.

Done!

Hey @crusj. Great to hear about the new storage_dir field. I tried setting the storage path to ~/.config/storage/nvim/bookmarks locally via storage_dir, but that didn't seem to work.
require("bookmarks").setup({
  storage_dir: "~/.config/storage/nvim/bookmarks",
  // ...
})

Hey @crusj. Great to hear about the new storage_dir field. I tried setting the storage path to ~/.config/storage/nvim/bookmarks locally via storage_dir, but that didn't seem to work.
require("bookmarks").setup({
  storage_dir: "~/.config/storage/nvim/bookmarks",
  // ...
})

You should use absolute path.


ActiveSupport >=5.1 Support

Adds support for active support 6, whilst maintaining compatibility with active support 5 and older ruby versions.

Did you see the #45?

Did you see the #45?

Yes I did, the end result of this PR is the same (allow this Gem to be used in Rails 6 projects) however it maintains support with older rails / rubies which seem to be the main objections to merging the referenced PR

@tmlee This project still maintained?

@tmlee This project still maintained?

You can switch to the replace_activesupport_with_tardis_and_dry branch of my fork, if you wish.

@tmlee This project still maintained?

Yes i will update this gem soon to support newer version of Rails

@tmlee Please, that would be great!

I've chosen this version of code to be used.

I've chosen this version of code to be used.

Thank you for let us knowing!

To make it easier for those wishing to use @jmortlock's fork that adds Rails 6 support, you can use this in your Gemfile:
gem "time_difference",                          # Simple TimeDifference operations. NOTE: We use this fork to support Rails 6: https://github.com/tmlee/time_difference/pull/48
    github: "jmortlock/time_difference",
    ref:    "74cfe3fdbf2a50dd80d7c0fdf457bf2b9384edd6"

This locks it to that specific git commit as well to avoid any surprises with bundle update, should the master branch see any further commits.


[Event Change] Extending Service Contract Header Invoice Period not possible

Calculated output variable InvPeriodDuration in Event OnCalcInvPeriodDurationCaseElse(var ServiceContractHeader: Record "Service Contract Header"; InvPeriodDuration: DateFormula) of  Service Contract Header not declared as var therefore not given by Reference and the needed date formula will not be returned to the caller.

Thanks for reporting this. We agree, and we’ll publish a fix asap, either in an update for the current version or in the next major release. Please do not reply to this, as we do not monitor closed issues. If you have follow-up questions or requests, please create a new issue where you reference this one.
Build ID: 49264.


Binary patching of build prefixes

To-do list:

[ ] Add a test that raises an error if the patched binary is not the same number of bytes as the original.
[ ] Add a test that raises an error if there are any binaries (including those which are not ELF/MachO files) which still contain prefix references.
[x] Fix the Linux test bottle so it doesn't throw an error when trying to see if it has a cellar.
[x] Refactor skip_prefix_relocation to more closely resemble compatible_location? and use spec.cellar == HOMEBREW_CELLAR.to_us instead of Homebrew.default_prefix?
[x] Test this locally on macOS Intel in the prefix /opt/hb.

This PR implements part 1 of https://github.com/Homebrew/brew/issues/10846.  The goal here is replace references to the build prefix in a binary with users's prefix if the new prefix is of equal or shorter length to the build prefix.  If the prefix is shorter, strings which contain it are padded with null characters to make it the same length as the build prefix.
There are several things which I think need to be added to this PR, for which feedback would be greatly appreciated:

Proper error handling.  There are at least two scenarios where we might want raise an exception:
a) The patched binary is not the same length as the original.  The way the code is written right now makes this unlikely, but we still might want something in case of a weird fluke.
b) There should probably be an exception raised if, after patching, references to the build prefix still exist in binaries in the keg.
Given that this is an experimental feature which must be enabled with the environment variable HOMEBREW_RELOCATE_BUILD_PREFIX, I think for now exceptions we raise from this should just halt the bottle pouring and tell the user to unset that variable and build the formula from source.

I'd like to get some more clarification on two scenarios where a bottle is not relocatable which don't seem to be related to build prefix references in binaries:
if keg_contain?(HOMEBREW_LIBRARY.to_s, keg, ignores, formula_and_runtime_deps_names, args: args)
   relocatable = false
end

This looks it is checking for references to HOMEBREW_LIBRARY, which is being pulled from the environment and not something baked in to the bottle.  I'm not sure under what scenario this reference would show up in a keg (even if only in text files), but it doesn't seem like something that can be fixed with binary patching.
relocatable = false if keg_contain_absolute_symlink_starting_with?(prefix, keg, args: args)

This also appears to be an issue that is unrelated to build prefix references in binaries.  I'm also unsure of the circumstances under which this occurs.

In terms of testing, I've tried about a dozen non-relocatable formulae (and a few relocatable ones to make sure they're ignored) on Linux, and so far the binary strings look to be properly replaced and the formulae work with brew test.  I will have to set up a second Homebrew install in /opt/hb on my Intel MacBook for macOS testing because my main installation is in /usr/local.   I have tried manually running the core patching code on some binaries in irb on macOS, and it doesn't throw any errors.
Ideally we could automate testing of all non-relocatable bottles on each platform in a shorter, non-default prefix (i.e. /opt/hb), but I'm not sure how practical that is right now.  As a starting point it would be great to test some of the most popular non-relocatable formulae.

Review period will end on 2021-12-09 at 08:26:43 UTC.

What was part 1?

Part 1 is to build non-relocatable bottles with a very long install prefix (probably 255 characters like Anaconda does) so that we can use this binary patching strategy for any prefix 255 characters or shorter (which should cover the vast majority of prefixes used).

👍🏻 to raising exceptions for both of these. For b): this seems very likely considering we're not handling all the possible build prefix subdirectories (yet).

I will add some new errors to keg_relocate for those two scenarios.

This is a reference to Homebrew/brew code e.g. shims. I think it could potentially be fixed with binary packaging but should basically never happen anyway (I think we throw errors at bottle time?).

Ah yes that makes sense and we do throw an error if shims are detected during bottling.  For now I think I will leave this as is and we can see if any bottles actually fail to relocate because of references to the old HOMEBREW_LIBRARY which are not shims.

This seems like something perhaps worth playing around with a Linux/macOS GitHub-hosted actions worker to do?

That would be awesome.  I am not sure how difficult it is to set up a Homebrew install in an alternate prefix in GitHub Actions but if that's feasible then we could test everything once this is merged.

Part 1 is to build non-relocatable bottles with a very long install prefix (probably 255 characters like Anaconda does) so that we can use this binary patching strategy for any prefix 255 characters or shorter (which should cover the vast majority of prefixes used).

Ok so this hasn't been done yet, right? Might want to call this PR part 1 and that part 2 if so 😁

Ah yes that makes sense and we do throw an error if shims are detected during bottling. For now I think I will leave this as is and we can see if any bottles actually fail to relocate because of references to the old HOMEBREW_LIBRARY which are not shims.

👍🏻.
Pretty sure I added this logic to handle the case where you're installing an old bottle with this HOMEBREW_LIBRARY reference and we want it to be non-relocatable.

That would be awesome. I am not sure how difficult it is to set up a Homebrew install in an alternate prefix in GitHub Actions but if that's feasible then we could test everything once this is merged.

Just a question of time, really. You'll either need to move it around or do a manual install but should be minimal compared to testing time overall.

Review period ended.

Ok so this hasn't been done yet, right? Might want to call this PR part 1 and that part 2 if so 😁

Done!

Pretty sure I added this logic to handle the case where you're installing an old bottle with this HOMEBREW_LIBRARY reference and we want it to be non-relocatable.

Ah so maybe we didn't always throw errors about shim references in binaries.  If we end up replacing all instances of HOMEBREW_PREFIX as I mentioned above, this scenario would be covered anyway.

Just a question of time, really. You'll either need to move it around or do a manual install but should be minimal compared to testing time overall.

Good to know.  Maybe just to get this PR merged I'll try to test the 50 most popular non-relocatable formulae locally so that we know nothing really serious has broken and then we can move on to testing everything.

I pushed a version now which just replace the prefix.  Given that this is an experimental feature, I'm leaning towards just doing this replacement and seeing if anything breaks.  The tests I've run so far haven't had any issues.  The code is a lot simpler, and it also means we can write a simpler test after patching which will basically just check if any binary files which are not ELF/MachO still contain prefix references.

I'll review this properly tomorrow (sorry, been busy the last couple days).

Part 1 is to build non-relocatable bottles with a very long install prefix

I'm wondering if we should do that at the same time given there's likely going to be a number of false positives relating to /usr/local in particular that could have undesirable impacts (e.g. a software that places something in /usr/local in a remote Linux server).
I'm perhaps being overcautious but it's worth noting.

I'm wondering if we should do that at the same time given there's likely going to be a number of false positives relating to /usr/local in particular that could have undesirable impacts (e.g. a software that places something in /usr/local in a remote Linux server).
I'm perhaps being overcautious but it's worth noting.

Yes, I'm concerned to some extent about /usr/local for Intel macOS.  In the worse case scenario we'd have to limit this to Linux and ARM macOS because /home/linuxbrew/.linuxbrew and /opt/homebrew won't be used by anyone but us.  Since this will only work for non-default prefixes which are of equal or shorter length to the default prefix and /usr/local/ is already pretty short, I do think the probability of someone having a non-default prefix on Intel macOS that is short enough and happening to run a package with one of those false positives is probably very low.
I don't think we can really start shipping the long prefix bottles until we are certain that a given bottle can be relocated because once we ship that bottle, the build prefix must be patched for all users, even if they are in the default prefix (unless we want to build two bottles for each platform for non-relocatable bottles).  It's a bit of chicken or egg problem and I was hoping we could ship this and test all the non-relocatable bottles in some short prefix like /opt/hb first and then we'd know they'd be relocatable.

Good to know. Maybe just to get this PR merged I'll try to test the 50 most popular non-relocatable formulae locally so that we know nothing really serious has broken and then we can move on to testing everything.

As long as all the behaviour (at bottle and at pour time) is guarded by an environment variable: 👍🏻.

I'm wondering if we should do that at the same time given there's likely going to be a number of false positives relating to /usr/local in particular that could have undesirable impacts (e.g. a software that places something in /usr/local in a remote Linux server).
I'm perhaps being overcautious but it's worth noting.

Yeh, I've noted this above: you're not being overcautious and you're right here about /usr/local.

No. If you're in /usr/local a lot of stuff refers to /usr/local in a way that doesn't relate to it being the Homebrew prefix e.g. CMake will do lookups for files in /usr/local regardless of where you've installed it with Homebrew. This doesn't apply to e.g. /opt/homebrew.
Yeh, I've noted this above: you're not being overcautious and you're right here about /usr/local.

Based on this feedback, I'm wonder if we can adjust how aggressively we substitute based on the original build prefix.  If the original prefix is not /usr/local, we would just substitute all instances of HOMEBREW_PREFIX because the other prefixes we use (/home/linuxbrew/.linuxbrew and /opt/homebrew) are Homebrew specific.  If the prefix is /usr/local/, then we could only replace strings in an allowlist.  I'd suggest for now:
/usr/local/Cellar
/usr/local/Homebrew
/usr/local/opt
/usr/local/var/
/usr/local/Frameworks

The first two directories are Homebrew-specific, and the later 3 are not part of the standard FSH spec: https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s09.html and online references only ever seem to refer to Homebrew.  This would leave two directories unpatched whose presence is currently considered to make a formula non-relocatable: /usr/local/etc and /usr/local/share/vim.  /usr/local/etc is definitely used by other software judging by Google results, so I don't think we can patch that.  /usr/local/share/vim is technically valid under the FSH spec but I'm not finding any references to it being used outside of Homebrew, so it may be okay to include as well.  Thoughts on this?

If the original prefix is not /usr/local, we would just substitute all instances of HOMEBREW_PREFIX because the other prefixes we use (/home/linuxbrew/.linuxbrew and /opt/homebrew) are Homebrew specific.

Yep.

/usr/local/etc is definitely used by other software judging by Google results, so I don't think we can patch that.

We already relocate it in text files and have had no complaints about it so I wonder if there's any issue there.
Honestly, I think all our HOMEBREW_PREFIX subdirectories except bin, lib and sbin are fair game.
I think that poses the question though about what we do if those are embedded and they are actually, intentionally referring to HOMEBREW_PREFIX. Do we just write this off as non-relocatable?

We already relocate it in text files and have had no complaints about it so I wonder if there's any issue there.

Honestly, I think all our HOMEBREW_PREFIX subdirectories except bin, lib and sbin are fair game.

I think that poses the question though about what we do if those are embedded and they are actually, intentionally referring to HOMEBREW_PREFIX. Do we just write this off as non-relocatable?

This prompted me to re-examine what we actually replace in text files.  One of the relocations we define which I think is used in text files is
    relocation.add_replacement_pair(:prefix, HOMEBREW_PREFIX.to_s, PREFIX_PLACEHOLDER, path: true)

which implies to me that we would already be replacing all instances of /usr/local in text files.  If we're already doing that and we don't seem to have broken things then maybe it's not as risky to replace the prefix after all?
Of course I'm not opposed to taking the more cautious route here - I just want to make sure I understand what we already do in terms of relocation.

This prompted me to re-examine what we actually replace in text files. One of the relocations we define which I think is used in text files is
    relocation.add_replacement_pair(:prefix, HOMEBREW_PREFIX.to_s, PREFIX_PLACEHOLDER, path: true)

which implies to me that we would already be replacing all instances of /usr/local in text files. If we're already doing that and we don't seem to have broken things then maybe it's not as risky to replace the prefix after all?

Can you link directly to the code? It's hard to tell just from that snippet whether it's conditional or not.

Of course I'm not opposed to taking the more cautious route here - I just want to make sure I understand what we already do in terms of relocation.

Yeh, it may be that we're fine to just have an allowlist for stuff like clang, cmake that we actively expect to look in /usr/local so we can do something cleverer here.

Can you link directly to the code? It's hard to tell just from that snippet whether it's conditional or not.

Sure, this is the order of operations as I understand them:
When brew bottle is called, keg.replace_placeholders_with_locations is used to replace relocation pairs in text files and RPATH/install_names in dynamically linked binaries: https://github.com/Homebrew/brew/blob/d4481212984fe4aad9304cfdb0c0b39a4efa2e9e/Library/Homebrew/dev-cmd/bottle.rb#L397
keg.replace_placeholders_with_locations creates a Relocation object with prepare_relocation_to_placeholders:
https://github.com/Homebrew/brew/blob/d4481212984fe4aad9304cfdb0c0b39a4efa2e9e/Library/Homebrew/keg_relocate.rb#L107-L111
prepare_relocation_to_placeholders is where I pulled that replacement which replaces the prefix: https://github.com/Homebrew/brew/blob/d4481212984fe4aad9304cfdb0c0b39a4efa2e9e/Library/Homebrew/keg_relocate.rb#L88-L104
The Relocation object is then passed to replace_text_in_files, which calls replace_text for each file that needs it: https://github.com/Homebrew/brew/blob/d4481212984fe4aad9304cfdb0c0b39a4efa2e9e/Library/Homebrew/keg_relocate.rb#L142-L164
replace_text for a Relocation object just iterates through all relocation pairs stored in the object unconditionally: https://github.com/Homebrew/brew/blob/d4481212984fe4aad9304cfdb0c0b39a4efa2e9e/Library/Homebrew/keg_relocate.rb#L38-L51
My conclusion from going down this rabbit hole is that we do unconditionally replace HOMEBREW_PREFIX with a placeholder, and when the bottle is poured the placeholder is then replaced with the new HOMEBREW_PREFIX.

Yeh, it may be that we're fine to just have an allowlist for stuff like clang, cmake that we actively expect to look in /usr/local so we can do something cleverer here.

Interestingly cmake is already :any_skip_relocation and clang is :any on Intel macOS so these won't get any binary patching anyway.  Maybe it would be helpful for me to make a list of all non-relocatable formulae per platform, ranked by popularity.  That could help us identify any examples where we might need to be worried about references to /usr/local, and we can even check the binary strings in the bottle specifically for references to /usr/local/bin etc.

My conclusion from going down this rabbit hole is that we do unconditionally replace HOMEBREW_PREFIX with a placeholder, and when the bottle is poured the placeholder is then replaced with the new HOMEBREW_PREFIX.

Thanks for doing this ❤ . Yeh, that's my conclusion too. I agree with you that this unconditional relocation is probably the best idea until we get any complaints about behaviour changes that result.

Interestingly cmake is already :any_skip_relocation and clang is :any on Intel macOS so these won't get any binary patching anyway. Maybe it would be helpful for me to make a list of all non-relocatable formulae per platform, ranked by popularity. That could help us identify any examples where we might need to be worried about references to /usr/local, and we can even check the binary strings in the bottle specifically for references to /usr/local/bin etc.

Sounds like a great idea 👍🏻.
We can also probably consider doing something smart, manually at first and maybe eventually automatically, with Linux and/or ARM bottles to identify references to /usr/local in those so we know for macOS Intel bottles that they need to be careful for those cases.

Thanks for doing this ❤ . Yeh, that's my conclusion too. I agree with you that this unconditional relocation is probably the best idea until we get any complaints about behaviour changes that result.

That sounds good to me.  I will leave that part of the code as is for now and we'll see if breakage is observed.  The fact that we've been replacing /usr/local in text files for a long time seemingly without issue makes me hopeful that nothing too crazy will happen.

We can also probably consider doing something smart, manually at first and maybe eventually automatically, with Linux and/or ARM bottles to identify references to /usr/local in those so we know for macOS Intel bottles that they need to be careful for those cases.

I was also thinking this could eventually be useful if we do have delineate between pre-existing references to /usr/local and ones added from HOMEBREW_PREFIX.

Now that the core relocation strategy has been settled on I am going to consolidate all of the very helpful feedback so far into a to-do list in the main post.  Please feel free to suggest changes to that list!

I've been able to do a significant amount of local testing in both a CentOS 7 VM (prefix /opt/hb/.linuxbrew) and Intel macOS Monterey (prefix /opt/hb/), in which all non-relocatable packages tested (including glibc on CentOS 7) were patched.  After installation, I used fgrep to scan for remaining prefix references and these were the results:

The vast majority of the remaining matches in binary files in non-relocatable formulae in both OSes were .a static libraries.  I guess these are not included in the output of keg.binary_executable_or_library_files but this could be changed.  These are still ELF/MachO binaries and can be patched the same way.  But that poses a deeper question - given our general stance on static linking, should we even be shipping those files in the first place?  If not, should we audit for static libraries and make sure they are deleted?

In the CentOS VM, only found 5 cases of references to binaries in 121 non-relocatable formulae where were not ELFs:

glibc - the ld.so.cache contains prefix references, and it is must be regenerated in the post install with ldconfig or glibc will be broken.  This is actually the only other thing that is needed for the core functionality of glibc to work.  There's a few minor issues with locales that I'm still ironing out, though these have workarounds as well.
emacs - contains an emacs.pdmp database for caching configurations to decrease startup time - in Anaconda, this binary is patched with the usual null padding.
gnutls - contains a gnutls.go file which appears to be an ELF (probably compiled from guile) - I'm not sure why this was even skipped.  It appears to fail its test because gnutls.go needs to be patched.
ncurses - some of the terminfo files contain prefix references - Anaconda also patches these with null padding.
openjdk - contains a modules file which seems like some sort of database for caching libraries? - this isn't packaged by Anaconda, but appears to be a database that would be patched with null padding.  The directories it is referencing in the prefix also don’t exist.
texlive - this package (which I need to split up) contains the files tex.fmt, lollipop.fmt, mf.base and a luatex-cache file with prefix references.  These all seem to be caches/databases which would get the usual null padding treatment

In the macOS install, out of 69 non-relocatable formulae I see the same binary matches in the above-mentioned-packages (except glibc which is obviously Linux only).  In addition to those:

gobject-introspection contains prefix references to .typelib files.  These are G-IR files for glib that I believe are encoding library paths.  These don't get any prefix matches on Linux because it uses RPATH, but on macOS I think we do get prefix matches because install names are used instead.  Anaconda patches these on macOS with null padding.
emacs - some .elc files (byte-compiled elisp files) contain prefix references, but these are just in help strings that list off default search directories and are present in Linux as well.  I would consider these good examples of false positives
udunits - there is a .info file which includes /usr/local in a help string explaining where default installations go – also a false positive because the exact same string is found in Linux.
ruby - some .ri files contain references to /usr/local - these are documentation files and are also false positives because they are present in Linux.

I think the fact that we're only getting binary matches outside of ELF/MachO files for just a handful of formulae (keeping in mind the total number of non-relocatable formulae on our different platforms ranges from 644 to 913) is a good sign that most formulae won’t need any additional work to make them relocatable.  Furthermore, of the matches I mentioned, only gnutls appears to fail its test because gnutls.go was not patched.
My general thought on dealing with these special cases is that we should try whenever possible to regenerate the files in the postinstall as I was able to do with ld.so.cache for glibc, and only patch as a last resort if regeneration is too difficult or inconvenient.  Potentially we might want to condition such a step on the user being in a non-default prefix.
Lastly I will say that I've seen at least one consistent example of a failure in non-relocatable formulae due to prefix references in text files that were not replaced.  All 3 of the D compilers we ship (dmd, ldc and gdc) use a text config file to find some headers, and this file is not getting proper prefix substitution.  So there’s definitely more work that needs to be done for text replacement, but that can be tackled in a subsequent PR.

I guess these are not included in the output of keg.binary_executable_or_library_files but this could be changed.

Yeh, this feels like either another method or an optional, default-off parameter to this method.

But that poses a deeper question - given our general stance on static linking, should we even be shipping those files in the first place?  If not, should we audit for static libraries and make sure they are deleted?

No, I don't think so. They aren't causing us problems and deleting them is likely to break the workflows of others.

My general thought on dealing with these special cases is that we should try whenever possible to regenerate the files in the postinstall as I was able to do with ld.so.cache for glibc, and only patch as a last resort if regeneration is too difficult or inconvenient. Potentially we might want to condition such a step on the user being in a non-default prefix.

This makes sense to me. I also think this step should be conditional.
Patching should follow the usual policy of being submitted upstream and if upstream rejects/ignores it: we need to figure out another solution.

Lastly I will say that I've seen at least one consistent example of a failure in non-relocatable formulae due to prefix references in text files that were not replaced. All 3 of the D compilers we ship (dmd, ldc and gdc) use a text config file to find some headers, and this file is not getting proper prefix substitution. So there’s definitely more work that needs to be done for text replacement, but that can be tackled in a subsequent PR.

Good to know 👍🏻.

Great work here @danielnachun, things are going considerably better/faster than I expected them to!

Yeh, this feels like either another method or an optional, default-off parameter to this method.
No, I don't think so. They aren't causing us problems and deleting them is likely to break the workflows of others.

I'll see if there's a simple way to make keg.binary_executable_or_library_files return static libraries using a default-off argument.

Patching should follow the usual policy of being submitted upstream and if upstream rejects/ignores it: we need to figure out another solution.

What I meant by patching here was patching binaries, not code.  As in if a binary which is not an ELF/MachO and contains prefix references, should we try to regenerate it in the post install instead of patching it?
The one binary filetype I've seen which will need to be regenerated or patched in multiple formulae are the .typelib files I mentioned above.  They pull in full paths to libraries on macOS (probably from install names) but not on Linux.  It should be possible to extract whatever is used to generate these typelib files during the build process and rerun it in the post install.

What I meant by patching here was patching binaries, not code. As in if a binary which is not an ELF/MachO and contains prefix references, should we try to regenerate it in the post install instead of patching it?

Ah, gotcha. Personally, if it doesn't break anything and we're confident we're testing that path I don't see any issues with patching it. It'll be quicker, I'd imagine, that regeneration.

I'm still working on this and I'll give a quick update as to what's taking a while.  Based on our discussion about patching binaries which are not ELF/MachO files, I changed the strategy to find all prefix matches first, and then remove text files that don't need null padding.  I used the text_files method available in keg_relocate, which uses file to identify text files.  This mostly works, although it uses the "allowlist approach" of requiring the file description string to contain the word "text" within it.  Some text files like .json or .html don't include "text" in their description (some file types return the very unhelpful description data and nothing else ☹), so I've had to add a blocklist of additional file extensions and file basenames that should be considered text files even though file doesn't report them as such.
To make this approach work, I'm now testing all the non-relocatable formulae (on Intel macOS).  I've now tested about 70% of the non-relocatable formulae so far, and these are the issue I've encountered:

Some binary files that may need to be patched are being skipped.  This only happens to due to a part of the text_files method which will automatically add a file to the list of text files if its first line is a shebang.  One would assume that only text shell scripts would ever have this, but oddly enough some ocaml software has what appear to be binary shell scripts that start with a shebang (for calling an executable in  ocaml, not bash/sh).  I don't know if these need null patching but this wouldn't be hard to fix either way.

Some text files are mistakenly identified as binaries and get null padded patching.  As I mentioned above, this is easily fixed in most cases through blocklisting of file extensions and file basenames, but there are few text files that still slip through the cracks.  These are either files with extensions not in the extension blocklist (which I just add to the list), or files with basenames not in the basename blocklist.  This latter case is where things get tricky - while file extensions tend to be widely shared across many packages, file basenames are sometimes unique.  This may be the one case where we need to consider formula-specific blocklists of files that should be skipped for null padded patching.

I now have a handful examples of binaries (about 30 formulae right now) that sadly do seem to break from null patching.  So far these have mostly been less popular formulae, most of which have no dependents.  The way I'm currently determining this is by running the formula test, which is limited in some cases if the formula doesn't have a test (very naughty) or just calls --version or --help (less naughty), so it's possible there is other breakage out there that is obscured by poor tests.  We'll probably want to improve some of those tests, but for now that's not my priority.  The only upside of this finding is that it now gives me a set of formulae to test code for checking a blocklist of formulae that should be skipped for patching because they are known to break.  I'll have to do more testing on these formulae to confirm they are actually broken by patching and not for some other reason.  Assuming they are, I'll try to build them with Anaconda and see if they are broken there as well.  If some are still broken with Anaconda, I'll open an issue on their GitHub to see if they have any ideas about how to fix the issues.

For now my focus is on identifying and fixing the issues in #1 and #2, because these are issues with my implementation of this PR rather than of the structure of the binaries in the formula.  We'll eventually migrate the process of determining which files need patching to the bottling step, but that doesn't change the fact that we have to do this somewhere.
Hopefully for #3 we'll eventually identify ways to make all the non-relocatable strings patchable in the binaries (via compiler flags or by upstreaming changes to the source code), but for now I think we can just skip those formulae and focus on popular formulae with large numbers of dependents where we know the patching works.

Thanks for the update @danielnachun and nice work. I wouldn't worry about the long-tail of formulae for now and just try and get this merged and working on a selection of formulae we can iterate on/grow.

Thanks for the update @danielnachun and nice work. I wouldn't worry about the long-tail of formulae for now and just try and get this merged and working on a selection of formulae we can iterate on/grow.

In that case I'll focus on pushing what I have, and we can see what else is needed to get this merged even though we know some things will need to be fixed.  Since this is all gated by an environment variable anyway, I think printing a warning message that you are using an experimental feature and that breakage may occur should hopefully make it clear that this is a work in progress.
I think the only test that makes sense to add now that we try to patch all binary files is the one to make sure the size of the original and patched binaries match.  The one I had proposed before to check if there are still binary files with the prefix present doesn't really make sense to me because we try to patch everything and only skip files if they are in a blocklist of text files, which we know shouldn't get null padding.

Is there a better way to test for a text file than creating a list we match against? Do null bytes appear in text files?

oddly enough some ocaml software has what appear to be binary shell scripts that start with a shebang (for calling an executable in  ocaml, not bash/sh).  I don't know if these need null patching but this wouldn't be hard to fix either way.

I think these are sharballs, just like Anaconda's install script.

Since this is all gated by an environment variable anyway, I think printing a warning message that you are using an experimental feature and that breakage may occur should hopefully make it clear that this is a work in progress.

👍🏻

I think the only test that makes sense to add now that we try to patch all binary files is the one to make sure the size of the original and patched binaries match.

👍🏻

Is there a better way to test for a text file than creating a list we match against? Do null bytes appear in text files?

Yeh, I think we may have outgrown using file here. My understand is that in a pure text file: you shouldn't have null bytes. How we handle mixed text/binary files is more up for debate.

Is there a better way to test for a text file than creating a list we match against? Do null bytes appear in text files?

Yeh, I think we may have outgrown using file here. My understand is that in a pure text file: you shouldn't have null bytes. How we handle mixed text/binary files is more up for debate.

I hadn't thought of that before, but I think you are both correct - text files shouldn't contain null bytes.  I have some "messy" formulae that include text files that file doesn't think are actually pure text where I can test this.  If that finds the right binary files this could end up being a super simple test.  It's worth mentioning here that the text_files method I was calling is designed to generate an allowlist of pure text files that should get regular prefix replacement with no padding that is called by replace_text_in_files, not to make a blocklist of files that shouldn't get null padding patching.  Eventually we'll need to close this gap, but tackling that problem is a separate task from this PR

The suggestion to scan for null bytes was brilliant!  It took a bit to figure out the best way to implement it, but it solved all the weird corner cases I've encountered so far in terms of distinguishing between text and binary files.
The only quirky thing I discovered was that for BSD grep, one has to use egrep to search for "\x00" as a null byte, while for GNU grep, egrep (or the equivalent grep -E) does not work, and instead grep -Pa is needed.  I reused the recursive_fgrep_args method as those flags are also needed.  So the final BSD grep command for macOS is egrep -lrO "\x00", while for GNU grep on Linux it's grep -Pa -lr "\x00".
As I noted in a comment, it turns out that patching the sharballs (binary shell scripts) seems to break them, so I've deliberately used text_files to remove these from the list of possible matches.  This will be easy to change later if we figure out how/if sharballs even need to be patched.

Scanning for null bytes is a common heuristic. It's also what's used by ripgrep and bat, I believe. My only concern is that the things we're doing here are more destructive than what rg or bat do, so we might need something more than this (e.g. just like how we check for mixed text/binary files).

Scanning for null bytes is a common heuristic. It's also what's used by ripgrep and bat, I believe. My only concern is that the things we're doing here are more destructive than what rg or bat do, so we might need something more than this (e.g. just like how we check for mixed text/binary files).

Is there code somewhere in brew which already checks for mixed text/binary files?  I have been searching but haven't found anything yet.  I'm fine with skipping mixed text/binary files for now if we can reliably detect them.

Is there code somewhere in brew which already checks for mixed text/binary files? I have been searching but haven't found anything yet. I'm fine with skipping mixed text/binary files for now if we can reliably detect them.

Don't think so.

Sorry, I think I mislead you with my previous comment. I meant to write: "just like how we would need to check for mixed text/binary files".

Based on my testing so far, I've encountered a few cases mentioned above where patching seems to break something.  However there are no consistent patterns and in all the examples I've seen so far, the breakage happened when patching ELF/MachO and not some other unknown binary file.  My inclination is to get this merged as is because it works for the vast majority of the formulae, with the caveat that the feature is experimental and things can and probably will break.

@danielnachun just to be explicit: do you consider this ready to merge? If so: feel free (or let me know and I will)

It looks to me like more stuff needs to be gated by HOMEBREW_RELOCATE_BUILD_PREFIX. But other than that the basic shape of it makes sense to me.

I had to step away from this for a bit to deal with some work stuff, but I added the last thing I though was missing here: a very basic check to make sure the patched binary is the same size as the original.  It should never be too small because we use ljust to pad anyway.  However, it could end up being too big, and that would be bad because it would likely mean relocation was attempted into a prefix that was too long.  This shouldn't happen because we have other checks, but a little redundancy here can't really hurt.  I confirmed that the check fails I deliberately break it by padding the binary to be too large, and it passes if I don't try to break anything.
If the simple raise I used is considered sufficient, then I consider this ready to merge.  If a dedicated error class needs to be made, I can add that if desired.

Shoot I think this last commit I pushed actually doesn’t work as expected for the each_unique_file method I added.  Not sure what this might break now but I think we need to fix that ASAP.  The binary patching part is fine - the issue is that now each_unique_file_matching will probably error out.

Reverting in https://github.com/Homebrew/brew/pull/12870

I think that's good enough reasoning for me that this should have test coverage added before it's merged again, I'm afraid. Please also mark your PR as draft until you're ready for it to be merged.

Agreed on needing test coverage.  I will reopen but I’ll need some guidance on how to implement suitable testing here.  It will probably require setting up brew in alternate prefix in CI, which I’m not yet sure how to do.  I’ll start by studying how the current tests are implemented and build from there.

We should probably also aim for making any changes a no-op unless the appropriate environment variable is set.

Agreed.

I'll follow up further when I reopen this, but the breakage actually had nothing to do with the binary patching itself.  What broke was my attempt to reduce code redundancy between each_unique_file_matching and each_unique_binary_file, which both use grep to search files for strings.  So nothing new was being run - I just broke existing code which was being refactored.  We can be more cautious when I reopen this and simply leave the somewhat redundant code alone, but I do think it exposed that, even before my PR, each_unique_file_matching lacked both testing coverage and sufficient comments explaining how it works, all of which contributed to the breakage.

What broke was my attempt to reduce code redundancy between each_unique_file_matching and each_unique_binary_file, which both use grep to search files for strings. So nothing new was being run - I just broke existing code which was being refactored.

I'd suggest that you pull that change into a dedicated PR and ensure test coverage before this is merged.

probably require setting up brew in alternate prefix in CI, which I’m not yet sure how to do.

I don't think so. Unit tests would have caught this, I don't think we need end-to-end integration tests.


com.kiuwan.importer.beans.File will fail to hash source code correctly on different JRE/OS combinations

The bug is here:
https://github.com/kiuwan/thirdparty-report-importer/blob/master/src/com/kiuwan/importer/beans/File.java#L66
If the code is run on the same file on two different JRE/OS environments where the file encoding differs, the generated MD5 sum will differ as well.
The source code should be hashed with a given encoding; for instance, .getBytes("UTF-8").

Hi Francis. Thank you for reporting. We have just corrected right now.


Searching function is broken

Hey! Friend you can use this as an alternative for now.
https://github.com/nadia2436i/GetDLCInfoFromSteamDB2

For now you can open the file %localappdata%/GLR_Manager/config.json and change "use_steamdb": true to "use_steamdb": false

SteamDB updated their TOS and their Webpage with CloudFlare. Its also now against TOS (or probably has been this entire time) to scrape the SteamDB webpage. Using jiaaluo's alternative method or nitrog0d's changing of the use of steamdb is probably best.


Reportlet - Head-Motion+Eddy correction

Place under the viz module a reportlet generation tool, based on the comments here https://github.com/nipreps/dmriprep/pull/144#discussion_r561782596

We'd like to have some of the following:

Registration to anatomical before/after emc.
TSNR maps before/after and/or mean signal per pixel.
Time-series of registration parameters (i.e., xyz translation, rotation, shears...?).

I think the proper home for this is NiReports now, moving there.


Add client to the list of args send through to listeners

Description
Similar to #168, I'd like to propose that listeners (such as event, message, etc.) be able to access the @slack/web-api client. This would make it easier for listeners to make calls to the Web API methods while keeping the client rate-limit and queue-aware.
Without this, I either need to initialize my own web client in each file with listeners or pass app.client around manually
What type of issue is this? (place an x in one of the [ ])

[ ] bug
[x] enhancement (feature request)
[ ] question
[ ] documentation related
[ ] testing related
[ ] discussion

Requirements (place an x in each of the [ ])

[x] I've read and understood the Contributing guidelines and have done my best effort to follow them.
[x] I've read and agree to the Code of Conduct.
[x] I've searched for any related issues and avoided creating a duplicate issue.

I think this is a good idea. There's no reason that I can think of that app.client is not offered to the middleware and listeners in an application.

I agree this is a missing piece for common use cases.
Regarding the possible changes, are you thinking to add client and logger to listener args like https://github.com/slackapi/bolt/blob/%40slack/bolt%401.5.0/src/types/command/index.ts#L7-L14 or add them to context? I prefer the former.

Hey @seratch - definitely want to add client and logger as listener args, not context


Fix loop issue #54 (multiprocess python >3.8 on OSX).

I checked this in #55 and it seems that it does not close #54. However I'm fine with merging this as is because it doesn't seem to create more problems.


[BACKLOG-32494] - always using utf8 for serializing step meta

:white_check_mark: Build finished in 17m 42s
Build command:
mvn clean verify -B -e -Daudit -amd -pl engine

:ok_hand: All tests passed!
Tests run: 3627, Failures: 0, Skipped: 7    Test Results

:information_source:  This is an automatic message


force navigation does not work

I have a route with parameters.
        {
            name: 'test',
            path: 'test/:id',
            component: Test,
        },

If I use use:link and go from /test/1 to /test/2 it doesn't updates.

Could you provide a reproduction link and some more detail on the issue ?


Fancy zones will be reset to default zones on external monitors after sleeping

Microsoft PowerToys version
0.47.1
Running as admin

[X] Yes

Area(s) with issue?
FancyZones
Steps to reproduce

Create your own fancy zones layouts on external monitors connected to a Surface Dock.

Wait until the computer enters sleep.

Wake the machine

Try to move windows (new or existing ones) to external monitors and get them to snap to zones -- the default zones will be used.

Additional information:

Running as administrator or not does not make a difference
Versions before 0.47 had that bug, too but opening and closing the zone editor activated the selected layout. Now you have to restart PowerToys.
Opening the Zone Editor shows the selected layout. Changing the layout, leaving the editor, entering the editor and selecting the user defined layout does not help.
PowerToys have to be restarted.

Monitors have not been disconnected, the number of external displays did not change not were they moved while sleeping.
✔ Expected Behavior
All zones are where they were before. Windows should snap to the defined zones if moved appropriately.
❌ Actual Behavior
Windows snap to the default zones.
Other Software
No response

Duplicate of #13810

Thanks for reporting this. This issue is a duplicate. We'll track this in #13810 to centralize the discussion thread.


Wifi setup missing on screen keyboard characters.

The & (ampersand Alt+ 38) is missing from the Wifi setup's OSKB.
This is a nasty issue for me as the business I work for has several machines and devices that are frequently brought in but not long enough to catch the user and re-enter a new wifi key.
I wouldn't trust most of these users to make a peanut butter sandwich without the peanut butter let alone enter a new wifi key.
What would be a good workaround is if the wifi setup app could read a text file with the wifi SSID and key already there then delete the text file after asking the user for confirmation. This would only open up the wifi key to exploitation for a short window without having to add the ampersand and any other missing but valid wifi key characters to the keyboard.

You can use Retroarch to setup your wifi instead or use the alternative solution described here: https://github.com/christianhaitian/arkos/wiki/Frequently-Asked-Questions---rg351mp#q-how-do-i-configure-my-wifi-connection-in-arkos

Why not fix the keyboard?

I don't have the source code for that keyboard.  I'm hoping someone can provide a keyboard solution with source code.


Honor user-supplied environment variables in config.sh

Newsboat version (copy from newsboat -v): 2.10.1
It would be nice if config.sh would honor (i.e. write into config.mk so that make can use them later) environment variables that the user supplies, such as CXX, CXXFLAGS, LDFLAGS. (Or switch to CMake, autotools, or another standard configuration system that already has that feature.)

I struggle to imagine a use-case for that, can you describe one? If you need this feature, it means you're running ./config.sh in an environment that's different from the one where you compile stuff, which seems unusual to me.

I should mention that I am writing on behalf of the MacPorts package management system, where we have a port of newsboat. newsboat requires C++11. On older version of macOS that don't by default have a toolchain that supports C++11 (i.e. OS X 10.8 and earlier), we must supply a path to a C++11-capable compiler in CXX and a -stdlib flag specifying a C++11-capable C++ library in CXXFLAGS. MacPorts includes tooling that sets those environment variables at configure time, but not at build time, because ubiquitous build systems like autotools, cmake, etc. take those environment variables set at configure time and store them in files for use at build time.


core: Remove unused CModule variable

Removes unused CModule var. CModule crashes when you give it a dll tha's been loaded as one of these: LOAD_LIBRARY_AS_DATAFILE, LOAD_LIBRARY_AS_DATAFILE_EXCLUSIVE, LOAD_LIBRARY_AS_IMAGE_RESOURCE. We have guards for this in libsys but not here.
Idk if this'll fix the issue discussed on Discord but well see

Seems to fix the issue caused by #780 as per https://discord.com/channels/920776187884732556/942505193893945394/1278043205081890940


Automated cherry pick of #3661

Cherry pick of #3661 on release-5.16.

#3661: MM-17123 - Updating inline images markdown

/cc  @asaadmahmood

@asaadmahmood Please help merge.


Nodejs / add npm install npm@latest -g

Description
add npm install npm@latest -g to all images to prevent install errors like:
Your cache folder contains root-owned files, due to a bug in npm previous versions of npm which has since been addressed.
All Submissions:

[x] Have you ensured there aren't other open Pull Requests for the same update or change?
[x] Have you created a new branch for your changes and PR from that branch and not from your master branch?

Why not just npm cache clean --force if the cache directory permissions are wrong? FYI The images now contain two layers of installing npm@latest.

Why not just npm cache clean --force if the cache directory permissions are wrong? FYI The images now contain two layers of installing npm@latest.

because this was a bug in npm

I don't see how this fixes anything. There's npm -g install npm@latest in the layer above this. Now there are two layers of installing npm? This is not a NPM version error.
It's a misleading error whenever npm doesn't have permissions. Any error with EACCES or EPERM regarding cache dir will trigger it.  Error message itself is referring to 2018 for npm 8.12 where the referenced bug was actually implemented. The debug logs would show more, but it's most likely trying to make a hard link of the installed package for the cache.

example simulated error (not in pterodactyl)
npm verb stack FetchError: Invalid response body while trying to fetch https://registry.npmjs.org/corepack: EACCES: permission denied, link '/data/data/com.testfolder/files/home/.npm/_cacache/tmp/9b99c620' -> '/data/data/com.testfolder/files/home/.npm/_cacache/content-v2/sha512/8b/29/d8d04509b38b390ebcba4956b8fb2955f88570f48241edffb01128b2dcdfb4580c453db04a422a8046fb9fdf7c7c9c31a25a64ac280c8f7892511b0798a7'

I don't see how this fixes anything. There's npm -g install npm@latest in the layer above this. Now there are two layers of installing npm? This is not a NPM version error.
It's a misleading error whenever npm doesn't have permissions. Any error with EACCES or EPERM regarding cache dir will trigger it. Error message itself is referring to 2018 for npm 8.12 where the referenced bug was actually implemented. The debug logs would show more, but it's most likely trying to make a hard link of the installed package for the cache.
example simulated error (not in pterodactyl)

i think, i will be out here; you are doing nothing for months in here. Its a simple npm update which is needed

Discussion is healthy. You taking issues personally is not.
All this did was move the npm install to a new layer and increase the image size. The result is the same even if you move layers.


Make --insecure-registry option to be reconfigurable without restart

Due to issue comment https://github.com/docker/docker/issues/8887#issuecomment-214714813
As far as now there is no mechanism of working (adding, pulling) with insecure registries without docker and containers restarting, It would be useful if --insecure-registry option was added to the list of configuration changes that can be updated without restarting (https://docs.docker.com/engine/reference/commandline/daemon/#configuration-reloading)
Thank you in advance for considering this issue!

Thanks for opening!

/cc @justincormack I think you were also discussing this a while back, but I'm not sure what the outcome was of that discussion.

@thaJeztah there was a discussion about making it a docker run option but I can't find it right now. I don't think it would be very difficult to add it to the options that can be changed without a restart as it does not have to do very much; it was not in the daemon.json at all until 1.11 so I guess it was not considered until now.

Now, docker 1.11.0 supports 3 kinds of configuration in config reload:

debug
labels
cluster discovery
It is not very hard to support insecure-registry.


Page titles & HEAD displaying at the top of essays.html

A weird HEAD was being displayed at the top of essays.html. Essays, Git
Cheatsheet and Projects tabs displayed “Assignments” in the title.

The HEAD looks like it was from a resiidual merge conflict. The only problem is that now the line <link rel="stylesheet" type="text/css" href="./assets/styles/main.css" /> shows up twice in the file.
The rest of this PR looks good.

Thanks!


Support for typescript 4.6 moduleResolve: "node12" "nodenext"

I need tsd as tool to verify that types are working with all new module modes so i will add that as set able setting

Fixed by https://github.com/SamVerschueren/tsd/pull/149


Suggestion: extend .equal() with .equal.to() or equalTo()

Hi.
Please, assume that I did not have the chance to inspect the implementation details of this framework..
I am wondering if it is worth to consider (wheter it is possible) to add support for .to to improve readability of Assertion#equal and similars, as follows: true.should.be.equal.to(true).
Thanks,
Roberto

Sorry, this just increase number of internal keys to support. For me it does not make any sense to implement.


Remove request as a dependency

We have a dependency on https://www.npmjs.com/package/request , which is deprecated (and has been for some time).
To avoid everyone getting a warning, let's remove it.
I suggest replacing it with axios, but I have no strong opinion on it. I also haven't yet done a review of where we're using it.

Looks like we use it in the installer to download the ruby binaries dynamically, and in the service code to check if the Ruby Mock Service is up and running.
Axios is close enough to a default choice, and the project seems to have steered itself out of the wilderness, so is a safe enough choice I think.

Looks like we use it in the installer to download the ruby binaries dynamically, and in the service code to check if the Ruby Mock Service is up and running.
Axios is close enough to a default choice, and the project seems to have steered itself out of the wilderness, so is a safe enough choice I think.

Hello,
I'd like to work on this issue. It looks like just the two places @mefellows mentions in src/service.ts and standalone/install.ts.
Thanks

Amazing - thanks @adambocco .
I think @TimothyJones has made some progress/thinking on this. Tim - anything worth sharing?
If you're not already, please join slack.pact.io (and join the #pact-js-development channel) and we can chat there if you run into issues

Yes please, that would be excellent!
I took a stab at it before dinner yesterday, and found it a bit annoying to give proxy details to axios. It’s definitely possible, but might be more convenient to use a different library. I didn’t have anything worth saving- all I discovered is “this is more than 10 mins”.
Implementation wise and library choice, it’s up to you. I’d be happy to see any PR resolving the deprecation warnings

@adambocco Did you get anywhere with this? We're about to touch the installer code for a different reason, so if you have any work towards the solution it would be the perfect time to get it in.


Fixes and Extensions

Add paragraph about edge relationships (Section 3.4)
Add subsection about persistence to Docker section (Section 4.5)
Add params optional property bullet to POST-method query endpoint (Section 7.7)
Replace Python Postgres withpsycopg2 (Section 7.11)
More precise server call (Section 7.14)

Thanks @gramian !


improve popup and add description in new trip form expand icon

closes #205 & #208

(icons may be redundant here, added it as sometimes the popup covers the whole icon. but design ideas encouraged)

Instead of a reference to GPT what do you guys think about having it something like: "Speak your mind 🧠"

looks great! I especially like having the emoji in the popover.
a small nit, is it possible to make the popovers exit x have a bit more padding? rn it's kinda close to the edge

Updated popup


Example workflow - why use niche custom nodes in example workflows?

Hi.
Could you please change the default Text-to-video example?
I don't like to usually complain about free things, but the "Flux Prompt Enhance" node isn't very popular. It only has 62 stars:

Now, low star count does NOT mean this extension could somehow be malicious (because it is not very popular), but the repo of this custom node also says this:

So basically seems like the author has stopped also developing it.

Rather than complaining, I can give some suggestions. The sample workflow already uses String Function 🐍, which supports regular expressions, so no additional nodes are needed for replacing specific words .

"I can give some suggestions." - thanks but it is not like I can't replace it  (although in this case wouldn't bother to start fiddling with regex myself, as I barely remember anything about regex lol) I've installed too many custom nodes already. This seems to be a common trend with shared workflows on several sites. If it would be possible to avoid using some less popular nodes, then why not use the popular one instead? Even better, don't use any unnecessary custom nodes if possible, when sharing example workflows.

This advice is for the repo developers, they definitely have a reason to use these nodes, the pysssss nodes are almost a must. I'm just saying the examples could be improved a bit.


Added API endpoints for disconnecting Stripe

no-issue
This adds two new endpoints
DELETE /ghost/api/v3/admin/settings/stripe/connect
This will delete the stripe connect integration, but fail if there are any active subscriptions
GET /ghost/api/v3/admin/members/hasActiveStripeSubscriptions/
This returns an object with a hasActiveStripeSubscriptions boolean

Closed in https://github.com/TryGhost/Ghost/commit/4716d13d130c4967998b09b5356efcd8d4b2ebb8


[7.0.0] Cannot read property 'start' of undefined

I got notified of the new update of nyc 7.0.0 but I now get an error when running my tests.
/home/ubuntu/react-clickdrag/node_modules/nyc/node_modules/istanbul-reports/lib/lcovonly/index.js:32
        writer.println('FN:' + [meta.decl.start.line, meta.name].join(','));
                                         ^

TypeError: Cannot read property 'start' of undefined
    at /home/ubuntu/react-clickdrag/node_modules/nyc/node_modules/istanbul-reports/lib/lcovonly/index.js:32:42
    at Array.forEach (native)
    at LcovOnlyReport.onDetail (/home/ubuntu/react-clickdrag/node_modules/nyc/node_modules/istanbul-reports/lib/lcovonly/index.js:30:28)
    at LcovReport.(anonymous function) [as onDetail] (/home/ubuntu/react-clickdrag/node_modules/nyc/node_modules/istanbul-reports/lib/lcov/index.js:21:24)
    at Visitor.(anonymous function) [as onDetail] (/home/ubuntu/react-clickdrag/node_modules/nyc/node_modules/istanbul-lib-report/lib/tree.js:34:30)
    at ReportNode.Node.visit (/home/ubuntu/react-clickdrag/node_modules/nyc/node_modules/istanbul-lib-report/lib/tree.js:123:17)
    at /home/ubuntu/react-clickdrag/node_modules/nyc/node_modules/istanbul-lib-report/lib/tree.js:116:23
    at Array.forEach (native)
    at visitChildren (/home/ubuntu/react-clickdrag/node_modules/nyc/node_modules/istanbul-lib-report/lib/tree.js:115:32)
    at ReportNode.Node.visit (/home/ubuntu/react-clickdrag/node_modules/nyc/node_modules/istanbul-lib-report/lib/tree.js:126:5)
npm ERR! Test failed.  See above for more details.

Anything I should change in my setup?
My nyc config is this:
"nyc": {
    "sourceMap": false,
    "instrument": false,
    "reporter": [
      "lcov",
      "text"
    ]
  }

I'm also using the coverage plugin.

Updating all my deps + switching from the coverage for the new istanbul plugin resolved my issue.

@tleunen awesome, let me know if you run into any more issues. merging the Istanbul and nyc project was a lot of work (lots of code changed).

I'm getting
/Users/andy/crater/node_modules/nyc/node_modules/istanbul-reports/lib/lcovonly/index.js:32
        writer.println('FN:' + [meta.decl.start.line, meta.name].join(','));
                                    ^

TypeError: Cannot read property 'decl' of undefined


Add link to google play

Please, add GooglePlay link to README.md
https://play.google.com/store/apps/details?id=com.chdev.ks.minx

Done :)


working with  tags in nbsphinx

(all figures below were created from https://github.com/phaustin/nbsphinx_issue/tree/issue)

jupyter nbconvert is able to produce pretty good-looking html files
from notebooks using scaled <img> elements, e.g.

https://github.com/phaustin/nbsphinx_issue/blob/issue/python/img_fig_eq.py#L43
is converted to:
https://clouds.eos.ubc.ca/~phil/github/nbsphinx/nbconvert/img_fig_eq_nbconvert.html

nbsphinx workinng on the same notebook omits the image:

https://clouds.eos.ubc.ca/~phil/github/nbsphinx/img_fig_eq.html
but does work when I use a markdown image tag with a width attribute:
https://github.com/phaustin/nbsphinx_issue/blob/issue/python/md_fig_eq_aligh.py#L44
produces:  https://clouds.eos.ubc.ca/~phil/github/nbsphinx/md_fig_eq_aligh.html

Since jupyter notebooks ignore attributes like width, height for markdown images, at the moment I'm
producing two versions of each notebook:  one for jupyter ipynb files with <img> tags
and one for html nbsphinx conversion with ![altfig]() markdown tags.
Is there a way to get nbsphinx to handle img tags the way that nbconvert does?

What does nbconvert do when you convert your notebook to LaTeX/PDF?
Both nbconvert and nbsphinx use pandoc to parse the Markdown text, but I guess the difference is that nbconvert converts directly to HTML (or LaTeX), while nbsphinx converts to RST first.
There may be a way to force pandoc to keep the original <img> tag, but it would most likely not work for LaTeX output.
See also #52.

for future googlers -- this is the translation code I'm currently using to go back and forth between jupyter markdown and rst scaled images:
jupytext#305

similar issue here - are you planning on integrating support for images as html elements soon?

This should be possible soon using myst_nb.  This is very close to being a drop-in replacement for nbsphinx, although stil alpha.  (e.g. see /ExecutableBookProject/MyST-NB#98)

ok. I tried to understand your workaround (https://github.com/spatialaudio/nbsphinx/issues/284#issuecomment-515193285) , but I wasn't sure where that gist goes - could you elaborate?

I was doing a separate step where I post-processed the notebooks and replaced the img tags with the corresponding markdown.  That proved too cumbersome though, so I just started making a separate media folder with resized images.   My current approach is to keep the img tags and use chrome pdf print instead of latex to produce pdf:  see https://github.com/eoas-ubc/paged_trio

ok thanks. I'll keep an eye out for MyST-NB

@mluerig In case this is addressed to me:

are you planning on integrating support for images as html elements soon?

No, I'm not planning to work on this anytime soon.
But I'm open for contributions!

I got it to work by just using markdown for images inside my html div structure to scale, center and what not.
if I have the time at some point I'll get back to you

Does #438 solve this issue?

I think this is solved with the latest nbsphinx release (0.7.0).
Please leave a comment if something is still missing!


Generated library names won't pass linting with generate_from_arb

Currently there's no way to specify the library name that generate_from_arb produces. This means if you run the linter against a generated library that it will fail.
The analyzer was relaxed to allow library names to be omitted so that's one option. Another option is to have a flag that allows the library name to be prefixed. A further option is to read the pubspec and get the library name from that and then based on the path determine the library name based on the conventions.
The flag is probably the most straightforward of those options.

So what is it that causes the linter to fail? My thought would be that the library name should match the file name, so there doesn't need to be an additional option, it should just use the generated-file-prefix. Or is there something else that's a problem? Or we could just omit the name, I think.

Currently to run one of my arb files I do the following command after a pub install.
pub run intl:generate_from_arb --output-dir lib/src lib/intl.dart translations/en_GB.arb
The generated file has the following header.
/**
 * DO NOT EDIT. This is code generated via package:intl/generate_localized.dart
 * This is a library that provides messages for a GB locale. All the
 * messages from the main program should be duplicated here with the same
 * function name.
 */

library messages_GB;
import 'package:intl/intl.dart';
import 'package:intl/message_lookup_by_library.dart';

If the package name is my_intl then the library name should be my_intl.src.messages_gb when following the convention. Not sure if it should be my problem to make the translation filename be all lower case but that's another bit of bad behavior.

Another lint issue I see here @alan-knight is that if you had
"msgFridayVariable":"Friday, {time}",
"@msgFridayVariable":{"type":"text","placeholders":{"time":{}}},

The generated code is
static msgFridayVariable(time) => "Friday, ${time}";

When it should be
static msgFridayVariable(time) => "Friday, $time";

Can probably do some kind of regex around that.

There's also another point : if you have several libraries using .arb generated locale all of them will have the same name and conflicting.
To be honest I think this use case at the moment will apply only to me because I've patched the CompositeMessageLookup to allow for merging multiple messages (BTW this IMHO can be a feature request).

I filed https://github.com/dart-lang/linter/issues/165 , but the feeling there seems to be to use .analysis_options to exclude the generated code. But I'll remove the library directive anyway, I don't think it's serving any real purpose.
I'm not sure if excluding the files from analysis really works - would it mean that calls to initializeMessages would show as errors?
I think multiple independent libraries is a separate issue.

I'm not sure if excluding the files from analysis really works ...

It is kind of confusing. When you exclude files from analysis it's really saying "don't analyze these files unless you need to, and even then treat them as if they were external to my package" (similar to the way code from other packages is treated).

For the library name it's probably simplest to remove it. I don't think it's used for anything. Trying to figure out the name from the path seems poor. And what is it supposed to do when the library isn't in a package URL accessible location, either because it's not in a package at all, or it's in somewhere like /test.
It's probably excessive paranoia from too much dealing with XSS and SQL injection, but I prefer putting the ${} around names that are being added programmatically.
Library name removed in 0.12.6.

Is there any harm in adding the following comment at the top of the generated file, as suggested in a related issue in the linter?
// ignore_for_file: non_constant_identifier_names, omit_local_variable_types, unnecessary_brace_in_string_interps


chore: Improve yarn dev reliability

Closes #747
Issue

yarn dev only creates all.css.
But index.html imports global.css and anchor.css.
Thus, it gives the file not found error in #747 when running yarn dev without yarn build.

PR

index.html imports all.css instead of global.css + anchor.css

See old solution description

Added dev:setup: this builds (not watches) the three CSS files used by the index and component pages:

anchor.css & global.css: used by index.html
all.css: used by component pages

yarn dev runs yarn dev:setup before starting the vite server.
build:watch:scss now also listens for changes in global.scss

Maybe this PR could wait after we merge repos, things we need might change?

This PR now became a helper for the repo merge PR #868 (https://github.com/iTwin/iTwinUI/pull/861#discussion_r1052725440).
We could still do this PR after the merge repos PR, but would be good to have the hot reload for CSS working when the repos are merged.

This PR is replaced by #935 and can be closed.
@rohan-kadkol


Merge target specific postsubmit jobs into one

Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

/lgtm


[x] The website is reachable.
[x] The CNAME record doesn't contain https:// or /.
[x] There is sufficient information at the owner field.
[x] There is no NS Records (Enforced as of Sepetember 4th, 2024)

Description

I realized that I did the github verification thing on the wrong domain 🤦🤦🤦
IDK why i did that, but now I'm adding the github verification thing on the correct subdomain
How did I not see that 🤦🤦🤦🤦🤦
Link to Website

https://python660.is-local.org

sorry, dev-dibster 😭😭😭
I only just realized that I made that mistake


Rename core to api

Pull Request Etiquette

[x] I have checked the PRs for upcoming features/bug fixes.
[x] I have read the contributing guidelines.

Changes

[x] Internal code
[x] Library interface (affecting end-user code)
[ ] Documentation
[ ] Other: _____

Closes Issue: NaN
Description
Rename core to api.

I feel like you just did a search and replace for net.dv8tion.jda.core :P

Most likely a refactor on the package in IntelliJ instead


Fix Conductor OSGi Bindings

This patch fixes the OSGi bindings of Opencast's conductor module which
were accidentally removed in pull request #3255.
https://github.com/opencast/opencast/pull/3255/files#diff-6903fd0392932ec6f36f3e69d22808c75924bff91b26321b1a0b0d247370a2d7
With patch applied:
karaf@root()> bundle:list |grep conductor
 80 x Active   x  82 x 12.0.0.SNAPSHOT            x Opencast :: conductor
karaf@root()> bundle:services 80

Opencast :: conductor (80) provides:
------------------------------------
[org.osgi.service.cm.ManagedService]
[org.opencastproject.event.handler.AssetManagerUpdatedEventHandler]
[org.osgi.service.cm.ManagedService, org.opencastproject.event.handler.OaiPmhUpdatedEventHandler]
[org.opencastproject.event.handler.ConductingEpisodeUpdatedEventHandler]
[org.opencastproject.event.handler.WorkflowPermissionsUpdatedEventHandler]
[org.opencastproject.event.handler.SeriesUpdatedEventHandler]
[org.opencastproject.event.handler.ConductingSeriesUpdatedEventHandler]
karaf@root()>

Your pull request should…

[x] have a concise title
[x] close an accompanying issue if one exists
[x] be against the correct branch
[x] include migration scripts and documentation, if appropriate
[x] pass automated tests
[x] have a clean commit history
[x] have proper commit messages (title and body) for all commits

Thanks for the review!
Merging…


Manual ground plane doesn't work

Hello,
I'm trying to set up the system with two Intel Real Sense cameras. Right now I am stuck at getting a single one to work, though.
I'm running a single_camera_tracking container from docker, and it launches fine, but the window for the manual ground plane estimation doesn't work. It shows up (with the title "pick 3 points"), but no gui elements like in the guide. I can set three (or more) points, but nothing happens when I try hovering over them holding the shift button. Am I doing something wrong or does the software not work right?
Thanks and kind regards

Hi VanDerSloth! I don't have a setup right now but I know this was working when I last used the system- when using manual ground plane the window will stay open, but should gray out after a few moments (as seen here). If it's not graying out, you might also try holding the 'control' button instead of shift (both shift & control are used to complete the manual selection). Hope this is of some help!

Hi VanDerSloth! I don't have a setup right now but I know this was working when I last used the system- when using manual ground plane the window will stay open, but should gray out after a few moments (as seen here). If it's not graying out, you might also try holding the 'control' button instead of shift (both shift & control are used to complete the manual selection). Hope this is of some help!

Hi, thanks for your reply and sorry for my late reaction, I wasn't at my workplace since that time and could only now try it out. Sadly, holding control doesn't change anything. The window also won't gray out or anything, I can only keep adding red dots to the still image.


unable to prodcude bing_bert with nvidia data

I used the default setting and run the code with 32 V100, the data was constructed from nvidia scripts.
I was able to reproduce nvidia's results on Squad (F1=90), but failed to reoroduce deepspeed results.
stage 1, maxlen 128 (epoch 0-16)

stage 2, maxlen 512 (epoch 17-20)

I evaluated the epoch20 checkpoint on BingBertSquad, and the results were unexpected low:
F1=21
Thank you .

@jeffra @tjruwase

@1024er, apologies for the delay. Will take a closer look asap.

@1024er, apologies for the delay. Will take a closer look asap.

Thank you so much ~ Is there any progress please? ：）

@1024er, apologies I have not had much time to explore this.

@1024er, apologies I have not had much time to explore this.

Hi，do you have time to take a look recently ? Thank you @tjruwase

This is due to the inconsistent amount of data  in the shards used by each gpu. This problem can be solved by processing the same amount of data per shard.

Hi @1024er , have you ever encountered the problem that DeepSpeed will stall at the end of the first epoch (please refer to this issue)?
If so, I wonder if it is convenient for you to share your solution? If not, could you please tell me the DeepSpeed version and the Nvidia docker version you used in your experiments?

This is due to the inconsistent amount of data on each gpu. This problem can be solved by processing the same amount of data per shard.

Hi @1024er , have you ever encountered the problem that DeepSpeed will stall at the end of the first epoch (please refer to this issue)?
If so, I wonder if it is convenient for you to share your solution? If not, could you please tell me the DeepSpeed version and the Nvidia docker version you used in your experiments?

This is due to the inconsistent amount of data on each gpu. This problem can be solved by processing the same amount of data per shard.

Thank you for your reply. I'll have a try and share my results asap.

Hi @1024er , have you ever encountered the problem that DeepSpeed will stall at the end of the first epoch (please refer to this issue)?
If so, I wonder if it is convenient for you to share your solution? If not, could you please tell me the DeepSpeed version and the Nvidia docker version you used in your experiments?

This is due to the inconsistent amount of data on each gpu. This problem can be solved by processing the same amount of data per shard.

Thank you for your reply. I'll have a try and share my results asap.

Have you encountered the problem with a very low F value mentioned above ?

Hi @1024er I'd suggest to double check whether the checkpoints are loaded successfully or not by checking the return values of the model.load_state_dict calls in https://github.com/microsoft/DeepSpeedExamples/blob/master/BingBertSquad/nvidia_run_squad_deepspeed.py#L828
There should be 2~4 missing keys depending on whether you use LM or not. If you see more than that, that's the problem

Hi, @1024er . Sorry for late reply. I've tried your method in the last few weeks. However, DeepSpeed still have some other bugs such as optimizer loss becomes NaN in fp16 mode.
I'm not working on this framework because I can't reproduce DeepSpeed's BERT results for almost two months. However, from my experience, I have tried Megatron-LM (which is also part of the code source of the DeepSpeed) For your reference, the latest version of Megatron-LM code can't reproduce the results on the SQUAD dataset either (its accuracy can only reach about 60% with BERT-base).
Good luck!


ci: attempt to report coverage change as a comment on PRs

Change Summary
Adds a CI step to report coverage changes as a comment on PRs. Based on https://github.com/marketplace/actions/python-coverage-comment, in turn referenced at https://coverage.readthedocs.io/en/latest/other.html#reporting-helpers.

Related issue number
Addresses https://github.com/pydantic/pydantic/pull/9740#issuecomment-2186698277.

Checklist

[x] The pull request title is a good summary of the changes - it will be used in the changelog
[ ] Unit tests for the changes exist
[x] Tests pass on CI
[ ] Documentation reflects the changes where applicable
[ ] My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

The solution is not working at the moment (still trying to understand why...).  This is probably because this is an external PR, so it's expected. Ensure you have an additional `workflow_run` step configured as explained in the documentation (or alternatively, give up on PR comments for external PRs).
Notice: Ending action

the coverage reported within the python-coverage-comment-action artifact does not coincide with the official one (97.22% vs 96.33%)

On a (completely unrelated) side note, I have been experimenting an issue when running make test locally since the addition of the fail-fast feature; this is strange as CI tests on Windows do pass. Failures (all in test_fail_fast) are similar to the following but do happen to all parametrizations but the field-default related ones. Anyway, I can open a dedicated issue.  I'll try opening a PR with your changes and see if we can get things working!

On a (completely unrelated) side note, I have been experimenting an issue when running make test locally since the addition of the fail-fast feature; this is strange as CI tests on Windows do pass. Failures (all in test_fail_fast) are similar to the following but do happen on all parametrizations but the field-default related ones. Anyway, I can open a dedicated issue. I was just wondering if anybody could reproduce (on Windows)

Do you have v2.8.0 installed? You might need to upgrade your pydantic-core version. Lmk how that works for you!

Also, I'm guessing we don't need the coverage.yml file assuming the added step in ci.yml works?

Closing in favor of https://github.com/pydantic/pydantic/pull/9833, great work here!

Also, I'm guessing we don't need the coverage.yml file assuming the added step in ci.yml works?

Hi @sydney-runkle, the two files approach should be the way to go (according to the docs at https://github.com/py-cov-action/python-coverage-comment-action?tab=readme-ov-file#basic-usage)

for cases where you expect PRs from users that don't have write access to the repository

Btw, I saw you kept the approach on the new PR :)


feat: Omit specific headers by default

Purpose
Alternative to https://github.com/seek-oss/logger/pull/84
Pino has a limitation where you can either redact or remove specified redact.paths by setting redact.remove to true or false.
In some cases, it's reasonable to want to redact some properties e.g. headers.authorization so you know that you have an authorization header but in other cases, the header, e.g. x-envoy-peer-metadata only contributes to increasing the log message size and therefore cost.
To reduce costs, we should be able to remove the properties that are not relevant for diagnostic purposes.
Approach

Properties containing headers that should be omitted were identified via this Splunk query over 1 business day:
index="*"
| fields *x-envoy-*
| fieldsummary
| rex mode=sed field=values "s~\[?\{\"value\":\"~~g
  s~\",\"count\":\d+\}[,\]]*~~g"
| eval avgBytes=round(len(values)/distinct_count,0), totalMiB=avgBytes*count/1024/1024
| table field, count, distinct_count, avgBytes, totalMiB
| sort 0 -totalMiB

The headers are typically under req, headers, and requestHeader properties.

Added defaultOmitHeaderNames which is a list of header names to omit.
export const defaultOmitHeaderNames = [
  'x-envoy-attempt-count',
  'x-envoy-decorator-operation',
  'x-envoy-expected-rq-timeout-ms',
  'x-envoy-external-address',
  'x-envoy-internal',
  'x-envoy-peer-metadata',
  'x-envoy-peer-metadata-id',
];

Add a omitHeaderNames logger option that defaults to the defaultOmitHeaderNames. If a consumer wants to opt out, they can supply an empty list or their own list instead.

A headers custom serializer omits the specified headers from the headers property of the logged object.

The existing req custom serializer also omits the specified headers from the req.headers property of the logged object.

Notes
💡 It may be helpful to review commit-by-commit.
Answers from discussion with @72636c

✅ What's the risk classification for this change since it affects much of SEEK?
Minimal Risk based on this being "...business critical given it ladders up to a citizenship ask (cost savings), has limited risk (only removes headers from logs that you’ve de-risked, pending point 3), and it’s still up to individual team discretion to review & choose to merge the version bump".
✅ There is no reqHeaders custom serializer so some guidance around this is needed. We could add this as it would be a clone of the headers custom serializer.
Since reqHeaders is produced by two services of one team, we will not be adding a specific custom serializer.
✅ Still following up with cloud platform team on these headers.
Cloud platform team is ok with this change and has had a look at this PR.
✅ Is it ok to remove the default export from serializers since it isn't used internally and I don't think it's accessible via e.g. import serializers from '@seek/logger/lib-commonjs/serializers'.
Removed default export from serializers.

Issues
Relates to https://github.com/seek-oss/logger/issues/83 but does not resolve it.

@72636c : I'm happy with the changes and learnt a few things too.


consistenecy: "User name" or "username"?

Inconsistency: Create an account field name is "User Name".  The green message says "The username" requested is available."  Is it 1 word or 2?

Both are technically correct but username is most common. (examples see twitter and github login screens)


Creating flashcards

In progress,
need to create and finish the handlebars for the flashcard routes,
flashcard routes needs to be finished

Need do the handlebars for flashcards


Some properties for RoundhouseSettings in Cake.Common.Tools.Roundhouse are not working properly

Error: C:/Examples/Ecommerce/blanqueria/Ecommerce/build.cake(92,4): error CS0117
: 'Cake.Common.Tools.Roundhouse.RoundhouseSettings' does not contain a definitio
n for 'Drop'
I need to drop the database using this command:
RoundhouseMigrate(new RoundhouseSettings{
ConnectionString = connectionString,
SqlFilesDirectory = "Ecommerce\Scripts",
Drop = false
});
Cake version: 0.15.2
Windows 10 64Bits

@ferminmoli Everything is there it just isn't working as you would expect.  The Drop Boolean in the settings file is internal. So you run the RoundHouseDrop command and it will toggle the Drop setting as part of the runner.  Look here at the RoundhouseDrop alias.:  https://github.com/cake-build/cake/blob/develop/src/Cake.Common/Tools/Roundhouse/RoundhouseAliases.cs
var runner = new RoundhouseRunner(context.FileSystem, context.Environment, context.ProcessRunner,   context.Tools);
runner.Run(settings, true);

So you need to run without the drop and it will not execute it:
RoundhouseMigrate(new RoundhouseSettings{
ConnectionString = connectionString,
SqlFilesDirectory = "Ecommerce\Scripts"});
As can be seen here:  https://github.com/cake-build/cake/blob/develop/src/Cake.Common/Tools/Roundhouse/RoundhouseRunner.cs
public void Run(RoundhouseSettings settings, bool drop = false)

Thanks for your help @phillipsj

Hmm, this feels like something that should be changed.

@patriksvensson, yes I'm agree with you. I can't figure out how to extend roundhouse functionality to pass more parameters. At the moment I can only run a simple migration and a Drop database but whole roundhouse functionality is encapsulated inside cake classes.

@patriksvensson @ferminmoli I agree it isn't intuitive, but having separate commands prevents mistakes as far as drop.  I have been wanting to learn more about roundhouse and I don't mind adding the additional that is needed and making that change if you want.

@phillipsj maybe it can be changed to accept manually parameters to take advantage of whole roundhouse functionality. I'm pretty sure that you have a better idea.

@ferminmoli  I will use some of the other implementations and post back and example for review.

@phillipsj thanks again for your contribution!


Augment SCOS System Test to Include Discovery UI

As a Smart City developer
I want automated test that cover my UI
So that I can have more confidence that my changes don't break things
ACs:

Once the existing SCOS system test complete, a test should execute that verifies

the existence of the newly created organization in org facets
filtering on that organization should allow us to verify the existence of the created dataset
clicking on the dataset takes you to the detail page for that dataset (determine what details to verify on this page)

If the UI test fails, SCOS system test should fail as it would today

Tech notes:

Consider use of cypress
Consider creating this test in a repo separate from scos-system-test so it can be used for future UI testing beyond SCOS system test.  We may want to add it to the react-discovery-ui repo or put it in a new repo.

Closed since it is redundant with the Cypress work