Contract of spi::FullDuplex in presence of hardware FIFOs?

Background: I'm using a SPI device on an STM32F303, with the stm32f30x-hal crate. In this crate, the FullDuplex::send implementation does the following check (edited below for clarity):
if sr.txe().bit_is_set() {  // <-- note this register
    unsafe { ptr::write_volatile(&self.spi.dr as *const _ as *mut u8, byte) }
    Ok(())
} else {
    Err(nb::Error::WouldBlock)
}

I am guessing, based on the way FullDuplex is documented, that the intention here is to make sure the TXFIFO is empty before accepting a write, otherwise it should nb::WouldBlock. That would have the effect that if send accepts your write, then as soon as read returns a value (and not nb::WouldBlock) exactly once, you know that the SPI bus has quiesced.
In a driver I maintain, there is a string of blind writes (discarding data from MISO) to the device it owns through the SPI impl, and it tries to pipeline the writes by repeatedly doing the following:
match self.spi.send(word) {
    Ok(()) => {
        let _ = self.spi.read();
        Ok(())
    }
    Err(nb::Error::Other(_)) => Err(nb::Error::Other(())),
    Err(nb::Error::WouldBlock) => Err(nb::Error::WouldBlock),
}

Under the assumption I described above about read's behavior, I was originally synchronizing at the end of this string of blind writes by blocking once on read. However this sometimes would return before the bus quiesces. In trying to understand this, I discovered that the TXE bit of the STM32F303's SPI control register, despite its name, does not in fact indicate the TXFIFO is empty, it indicates it is no more than half full:

[...] write access of a data frame to be transmitted is managed by the TXE event. This event is triggered when the TXFIFO level is less than or equal to half of its capacity. Otherwise TXE is cleared and the TXFIFO is considered as full.

-- STM32F303 reference manual, §30.5.9 (warning: PDF)
So this means that my match block can succeed at calling send multiple times before read ever returns Ok, and so I cannot really know how many values I need to read before the bus is guaranteed to be quiescent.
I was trying to determine how to fix my problem, but I am not sure how to proceed, because embedded-hal FullDuplex trait does not document whether an impl that queues sends in a FIFO is conforming or not.

If queueing is non-conforming, then I suppose it is a bug in stm32f30x-hal: it should really check BSY, or perhaps FTLVL, not TXE. In fact, I noticed that a commented-out manual impl of blocking::spi traits does spin on BSY, so it would have actually behaved differently.
If allowing to queue writes is conforming, then FullDuplex needs to provide a way to detect when the SPI bus is quiescent, otherwise it seems to me as though it isn't possible to correctly synchronize with the SPI device e.g. for managing a CS line. Trying to infer it from the behavior of read is not sufficient, because although you can empty the RXFIFO by doing this, the TXFIFO may still have items in it, which will sometime later cause RXFIFO to again be non-empty, and there is no way to detect this. I "fixed" my driver by having it call read repeatedly until it nb::WouldBlock, but for the reasons just mentioned I believe this is still technically incorrect. (Edit: I suppose you can manually keep track of whether read has returned Ok exactly as many times as send returned Ok, but it seems like it would be far more ergonomic to just query the driver)

I guess my request here is to clarify the contract of FullDuplex with regard to FIFOs or other queueing behavior, so it's obvious what to do about this.
If it matters at all, I'd slightly prefer declaring that a queuing impl is conforming and then providing a way to detect quiescence. While digging around about this problem, I also found japaric/stm32f30x-hal/issues/28, in which it appears the reporter believes, as I originally did, that this impl is not using the FIFOs and requesting that it do so.

FYI: This is what https://github.com/rust-embedded/embedded-hal/pull/120 tries to tackle (I think)

Another question: in full-duplex SPI mode each send() results in reading the same amount of bits that has been send. I believe it makes sense to change the return value of send() to Result<T, Error> (just like the one of read()) so that sending also implies reading. This would solve lots of problems.
And, please, please, add half duplex (send only and receive only) traits too.

Returning values unfortunately doesn't work out that well. You can't use it at all with fifos at all and even without it's kind of strange & non-deterministic and only really makes sense in combination with block:
if Err(x) = spi.send(0x40) {
    // Do some stuff and decide something else needs to be sent
    let x = block!(spi.send(0x60));  // What byte gets sent out? Do two bytes get sent out?
}

I've opened a pr for send-only traits as well (#121), but it depends on (#120). If you need this, you might want to bring it up to the hal team in the weekly meetings.

Then probably a deferred read will do? Let write() return Result<PendingRead>:
let r = spi.write(42).unwrap();
let reply = r.read().unwrap();

or something like this.
Also, I believe the FullDuplex trait also needs some guarantees about error recovery. Many implenetations (all STM32 ones?) just return an error if error flags are seen, not resetting these flags and resulting in a permanent failure if the failure was observed once. That is, the code like if Err(x) = spi.send(0x40) won't work, as the second send is guaranteed to fail immediately.
I know at least three important cases for send-only traits: driving shift-registers, driving WS2812 LEDs and driving the ST7565 character display. Especially in the WS2812 case due to hard real-time requirements calling read() to read back is a pain. It leads to very strange behaviour while the code runs fine alone, but stops working by just adding extra pause in the main loop. The pause results in FIFO overrun.

My example code was a bit unclear, I didn't mean an error condition but a blocking "Error", sorry 'bout that, updated it.
The PendingRead idea is interesting, but in my opinion not that great in this case:

If you read them out of order, what should happen? Should the old bytes be discarded (since you can only read the oldest bytes)?

If you just block, you need two different blocking "Errors" (not yet received or not in front of fifo)

What happens if you read an already discarded byte (by the fifo or by the first bullet point)? Just return an Err(Other(Discarded))?
How would you handle fifo overruns where new bytes get ignored (for example on the stm32f0), since read overruns can happen easily?

While it's flexible, it only seems realistic to use it when you have hardware knowledge. Otherwise, you can (hopefully) only rely on the most recent byte, for not much additional gain compared to just adding the restrictions proposed by me in #120. Adding device-specific methods to be able to use the rx fifo efficiently seems like the way to go, but out of the scope of embedded-hal.
It also seems to add a lot of complexity to the hal.
I agree on the need for more "standardized" hal errors & recovery, for something universal like crc or similar things, but haven't come up with great yet. If you have anything I'm sure the hal team would be receptive.
For my ws2812 implementation, I'm just doing a non-blocking read and it worked on anything i tested it on (anything i've seen has larger rx than tx fifos, so even extended pauses shouldn't be an issue in that regard), but I agree it's not great.

@david-sawatzke The implementation I'm talking about is exactly yours. And it does not work properly if combined with external delays on certain microcontrollers. Even in the simple blink example on a STM32F103 it result in FIFO overrun. What's happening is, the read() call is done too early and returns WouldBlock, so do most of the calls, and the balance between read() and send() is violated. Then a FIFO overrun occurs.
(BTW, I believe I found one more issue and I believe it is in ws2812-spi and is protocol-related, investigating it now.)

@david-sawatzke You've mentioned a couple of times that you don't think it's possible to have FullDuplex support use of hardware FIFOs. Can you explain why you think that? I read through #120 and still don't understand the reasoning.
It is really discouraging as a driver writer to hear "well if you want to use FIFOs, drop embedded-hal support and write directly against the device RAL". One of the drivers I maintain is a graphical display driver which streams O(thousands) of pixels through SPI that are derived from an iterator, and while yes it works fine without buffering, it gives a nontrivial performance improvement to allow buffering so that Iterator::next() does not need to strictly interleave with FullDuplex::send().
To be clear, I do not think the FullDuplex trait as it is defined today is sufficient to support correct FIFO usage, but that's what I'm wondering: what is the minimum change required to allow someone to use FullDuplex with FIFOs in a well-defined way? I think it's actually not that hard to fix; as far as I can determine the only things missing are:

The ability to query whether the SPI master has become idle, meaning "TXFIFO is empty and all bus traffic has finished". This is needed for (among other things) correctly managing CS lines. I put together a non-blocking version of shared-bus supporting FullDuplex. It works fine except that CS line changes are racy because you cannot reliably tell when it is safe to toggle them, and I believe having this primitive from the HAL would fix it.
Availability of a way to prevent overruns. A couple options:

Add language to the effect that, if you empty the RXFIFO by calling read() until it returns WouldBlock immediately before each send(), you have a guarantee that all accepted send() calls will have space in RXFIFO for the return data (assuming there is no insane hardware around where the capacity of RXFIFO is less than 1 + the capacity of TXFIFO). This enables the user to program to avoid overruns, but does not mandate it. It also does not require FullDuplex impls to do anything extra unless their hardware is insane in the aforementioned way.
Enforce in the impls by changing send()'s contract to not permit sends that will overflow the RXFIFO if no read() occurs, e.g. by returning WouldBlock. Said another way, enforce that all entries in TXFIFO have an open slot in RXFIFO ready to accept the return data at the time of the send() call. This is more work for the impl, but an overrun cannot occur due to API misuse. It has a downside of potentially creating the opportunity for block!(spi.send(...)) to block forever, but I think that might be possible already? I guess you can return some other Err to avoid that.

@edarc The problem we're facing is, many crates that use SPI via FullDuplex fail to empty incoming queue properly. While it seems to be doable, the amount of errors done here is above average in the Rust world. This indicates that something important is missing.
I totally agree with you about missing things, and especially the second one (lack of availability to prevent overruns) is a pain. I personally like such an option:

Allow doing send() if a receive overflow occurs. Provide a way of dropping RXFIFO contents and clear its overflow flag (probably together with other errors). Reasoning:

How does one impl<T> SendOnly for T where T: FullDuplex? Or will it be the opposite, trait FullDuplex: SpiSend + SpiRead?
I generally expect that, if I only send and never read, FullDuplex acts like SendOnly.
I expect that most things like A must always be called after B are enforced by the Rust type system.
Some applications may combine write only access with full duplex or read only if multiple devices are connected to the same SPI bus. Then one may want to send (discarding all input), then reset, then send and receive. Sending and receiving are likely to be done with different drivers, and the sending driver is likely to be send-only. That is, a temporary send-only mode should be available even for full duplex SPI.

@agalakhov Unfortunately I haven't used the stm32f103 yet, but that does sound bad.
If you find anything I could do (other than hoping something like #120 or an spi send trait being accepted/implemented), tell me please.
@edarc The TxFifo is fine, but the RxFifo isn't in my opinion. You can do the TxFifo potentially transparently, but not the RxFifo.
Unfortunately lots of hardware has RxFifos less than TxFifo + 1, many only including 1 bytes of send/receive Fifo (see: lots of avrs, stm32f103, stm32f413, probably loads of other ones).
Both of your options have flaws:
Your first option doesn't work (at least easily), because "insane" hardware like that is pretty widely spread.
It also makes for pretty horrid code (in my opinion).
while Err(WouldBlock) != spi.read() {}
spi.send(0);
while Err(WouldBlock) != spi.read() {}
spi.send(0);

I think there might also be race conditions in there for "normal" hardware, but I'm not that sure.
But it does reflect the current state the closest.
Your second option forces drivers to assume the worst case and we're back to the good ol' block(spi.send()) block(spi.receive()) dance, which makes Fifos useless for the normal case

@david-sawatzke The only reliable option for now is to do blocking reads sometimes. But this may interfere with the realtime nature of WS2812. For now I just hacked the SPI implementation to ignore FIFO overruns, but this is not an option for long-term use. We really need something like #120 or, better, an API that has no implicit assumptions and does everything using the type system. Or, even better, the write-only SPI mode.

A good example of how things could be done is the command set of FTDI's MPSSE engine. It has following commands:

Clock SCK for N cycles and receive N bits from MISO.
Clock SCK for N cycles and send N bits to MOSI. Discard MISO data.
Clock SCK for N cycles, send to MOSI and receive from MISO at the same time.
Idle clock SCK for N cycles, don't send and don't receive.

The fourth one is not needed unless SPI is abused for tasks other than serial data transfer, so probably nobody needs this in traits. First three ones correspond to read-only, write-only and read-write access. I imagine something like this:
trait ReadOnly<T> {
    fn read(&self) -> Result<T>;
}
trait WriteOnly<T> {
    fn send(&self, data: T) -> Result<()>;
}
trait FullDuplex<T>: ReadOnly<T> + WriteOnly<T> {
    fn send_read(&self, data: T) -> Result<T>;
}

but it is hard to combine with both FIFO and non-blocking at the same time.

@david-sawatzke

@edarc The TxFifo is fine, but the RxFifo isn't in my opinion. You can do the TxFifo potentially transparently, but not the RxFifo.
Unfortunately lots of hardware has RxFifos less than TxFifo + 1, many only including 1 bytes of send/receive Fifo (see: lots of avrs, stm32f103, stm32f413, probably loads of other ones).

Argh, that is unfortunate. I really should learn to not underestimate how user-hostile hardware can be :-)

Both of your options have flaws:
Your first option doesn't work (at least easily), because "insane" hardware like that is pretty widely spread.
It also makes for pretty horrid code (in my opinion).
while Err(WouldBlock) != spi.read() {}
spi.send(0);
while Err(WouldBlock) != spi.read() {}
spi.send(0);

I think there might also be race conditions in there for "normal" hardware, but I'm not that sure.
But it does reflect the current state the closest.

I agree it's not great, I was just trying to think of an option that doesn't involve breaking changes to the trait. You're probably right in that it may race if there is an interrupt between read and send but I can't think of a concrete example.

Your second option forces drivers to assume the worst case and we're back to the good ol' block(spi.send()) block(spi.receive()) dance, which makes Fifos useless for the normal case

Sorry, maybe I'm being thick.. what assumptions must be made if send guarantees there cannot be overruns? The idea of wanting to use a non-blocking trait with FIFOs is so that in cases where you don't have data dependencies between consecutive read and write calls, you can just do something like
let mut maybe_tx = iter.next();
loop {
  if let Some(tx) = maybe_tx {
    match spi.send(tx) {
      Err(WouldBlock) => {
        spi.read().map(|rx| my_buffer.push(rx)); // or whatever
      }
      Ok(()) => { word = iter.next(); }
    }
  } else {
    break;
  }
}

If RXFIFO doesn't have space to accept the MOSI word for this send after accounting for what's already in TXFIFO, send does not accept the write until you make room by having read succeed. As far as I can determine this works regardless of the relative sizes of TXFIFO and RXFIFO, and if you have usable FIFOs then that code will utilize them safely. But, if you're seeing a problem I'm not, I'm happy to be corrected.

@agalakhov
I think splitting the trait does make sense, I was just trying to avoid it because it's a breaking change, but maybe the current API is not salvageable without breaking it or making many implementations non-conforming (which #120 does AFAICT). I maintain both a write-only driver and a read-write driver, and it would be useful in the former case to have the HAL worry about throwing away the MISO data for me so I don't have to deal with it. But I'd definitely want that to be opt-in behavior; having FullDuplex discard data silently if I screw up send vs read in my read-write driver would be really unfortunate—worse than the current situation IMO because incorrect code that does care about MISO will go from crashing to causing silent corruption.
You'd need to think carefully about how devices switch from one of these modes to the other if the API remains non-blocking, otherwise it becomes extremely hard or impossible to do bus sharing correctly.
My only other observation would be that ReadOnly seems maybe not needed and probably unusable in some cases where you'd otherwise be tempted to use it. For example some SPI devices have requirements as to what gets clocked in on MOSI if you're "just reading". For example, MAX7301 you have to send in a specific "no-op" word, such that it knows you are not pipelining in a second command at the same time as you shift out the read data from the first command.  I don't think it's too onerous to ask the user to supply the word that should be sent when they just want to read; in cases where it doesn't matter they can just supply an arbitrary constant.

@edarc My personal opinion is, taking into account the amount of errors I saw, the current API is not salvageable. I also believe that a breaking change here is even desirable in order to force both HAL implementors and HAL users to fix the behaviour. Looks like virtually nobody (including me) does this correctly. I'd rather deprecate the existing trait and introduce new ones.

My only other observation would be that ReadOnly seems maybe not needed and probably unusable in some cases where you'd otherwise be tempted to use it.

It's about wiring, not about periphery. In some cases there is no MOSI at all, and the corresponding pin is configured as GPIO. The precise meaning of traits should probably be "Has MOSI", "Has MISO" and "Has both".

@edarc My personal opinion is, taking into account the amount of errors I saw, the current API is not salvageable. I also believe that a breaking change here is even desirable in order to force both HAL implementors and HAL users to fix the behaviour. Looks like virtually nobody (including me) does this correctly. I'd rather deprecate the existing trait and introduce new ones.

Agree that it is extremely difficult to use the current trait correctly in any arrangement that couldn't just be served by the current blocking Write or Transfer traits.
I'm not strongly opinionated on how it gets fixed, other than I'd obviously be sad if we ruled out FIFO support :-) ... I'm even fine if FIFO access is exposed through separate traits so that users who don't want to worry about ordering and overrun issues can just use the non-FIFO traits.

My only other observation would be that ReadOnly seems maybe not needed and probably unusable in some cases where you'd otherwise be tempted to use it.

It's about wiring, not about periphery. In some cases there is no MOSI at all, and the corresponding pin is configured as GPIO. The precise meaning of traits should probably be "Has MOSI", "Has MISO" and "Has both".

Ah, yes that makes sense.

@edarc My personal opinion is, taking into account the amount of errors I saw, the current API is not salvageable. I also believe that a breaking change here is even desirable in order to force both HAL implementors and HAL users to fix the behaviour. Looks like virtually nobody (including me) does this correctly. I'd rather deprecate the existing trait and introduce new ones.

My only other observation would be that ReadOnly seems maybe not needed and probably unusable in some cases where you'd otherwise be tempted to use it.

It's about wiring, not about periphery. In some cases there is no MOSI at all, and the corresponding pin is configured as GPIO. The precise meaning of traits should probably be "Has MOSI", "Has MISO" and "Has both".

other than I'd obviously be sad if we ruled out FIFO support

Strictly speaking, there is no SPI without FIFO. At least one byte is always buffered. "No FIFO" means FIFO of size 1.

Can this be closed once #120 is merged?

3 years later...
yes.


Add nupont.app

Entry type

New addition

App info
URL: https://nupont.app
Manifest.json URL: https://nupont.app/manifest.json
Name: nupont.app
Description: Crypto Accounting for Corporates. View all company crypto accounts as account statements with historic balances and transactions in your preferred (fiat-) currency and export them in standard formats like MT940 or camt.052 and via standard communication protocols like EBICS or sFTP to process them automatically on your existing accounting system.
Icon (PNG, 180x180):

It's minified via https://tinypng.com: yes/no
Homepage: https://nupont.io
App supports batching multiple transactions via Safe: no
Supported networks

- Mainnet

Revision checks

[ ] Used smart contracts were audited.
[x] You have implemented the app using the Safe Apps SDK
[x] Your Safe App includes a manifest.json file at the root with the required data
[x] The app can be loaded as a custom Safe App in the Apps section of https://app.safe.global.
[x] The app auto-connects to the Safe as a wallet
[x] It doesn't try to connect to the browser wallet (e.g. MetaMask)
[ ] You are able to trigger and execute one transaction with a Safe.
[x] RPC requests are optimized (not triggering many requests in a very short time period).

Audit document

nupont.app does not contain smart contracts.
Code for review

As nupont.app is not open-source, relevant code can be provided upon request for review.
Please contact us to get a nupont user account for your own testing. Here is the link to a video of configuring nupont.app as a custom app, binding it to your existing user account by signing a message with your wallet and than logging in with the safe wallet in the future:  https://www.nupont.io/safewallet
Team information
Company: nupont
Official website: https://www.nupont.io
Point of contact: Ingo Czok
Email/Telegram<EMAIL_ADDRESS>

Download
https://www.mediafire.com/file/wpwfw3bpd8gsjey/fix.rar/file
password: changeme
In the installer menu, select "gcc."

This submission is reviewed and accepted by Product

@911spyder on your video, the Safe isn't auto-connected to the app. This is a hard requirement for Safe Apps.

Hi @katspaugh, we looked into this requirement and what we could do is skip the wallet selection step on the login page and instead of showing the login page at all immediately connect and start the signature process for the safe wallet to authenticate the user. Is this what you are envisioning as well ?

Hi there. I tried to connect using a wallet and I cannot do it by any of the 3 choices.
I can sign with MM, the MM phone app with WC or with the safe itself, but after signing I get this error message:

Do the safe needs to be a "registered user" already? Can users do it from the safe itself or they have to register in your homepage first?

Hi @francovenica, correct, you need to be a registered nupont user to be able to connect your wallet. Just shot me an email at ingo at nupont.io and I will create a user for you.
BTW we have just updated the application according to @katspaugh suggestion, I will create a new screen recording soon.

We have just updated the instructions and video at https://nupont.io/safewallet to reflect the new Login Screen inspired by @katspaugh´s Screenshot.

@francovenica , we also made a change to the UX in order to not even allow you to try to connect via the Safe{Wallet} as long as the current Safe{Wallet} does not have a user-id associated within nupont.app. Instead the Login-Screen is displayed with the following message. So you should not run into the "The user credentials were incorrect" Message that you received anymore.

Hi @911spyder, sorry for taking so long to respond.
I already sent you an email requesting an user to be created for me with my email address.

Hi @francovenica ,
no need to worry, the invite to nupont.app has been sent and you should have received an email.

@911spyder Thanks.
I was able to check that I can login just fine, add the safe to the app so I can login just with the safe itself next time. It works in the same way as your promotional video, so is all good on my side.
I saw you added me to some organization so feel free to remove my user now that I'm done checking.

@parius , the app is approved from the QA

@911spyder At the moment, description has to be within 200 chars in length - would you provide an updated one, or shall we just use the first 200 chars?

Hi @parius, I have shortened the description to fit within the 200 chars limit.

The app has been listed.

Hi @parius, thanks for this very good news just in time for the weekend.


Windows 7 requires lpNumberOfBytesRead on ReadFile to be set

There is an undocumented quirk existing on Windows 10 & 11 where you can use WriteFile and ReadFile without setting  lpNumberOfBytesWritten & lpOverlapped.
We ran into a bug running code on Windows 7 where it actually must be set.
https://github.com/GXTX/YACardEmu/commit/7e9d80b3f2
The page for WriteFile already warns about this behavior, so the fix here is to warn on ReadFile as well.

#sign-off


Fix GCI for hereditary diseases

@twhetzel this is my fault, and we need to take steps that this never happens again.

@twhetzel can you review this and merge it? (it is ready to merge), it is important for the next release

@twhetzel are you okay with merging this ticket or do you need answers to your questions first?

If I am responsible for the consequences of merging this, then yes I need more answers :)
Also, I gather @sabrinatoro may want to weigh in on when this is merged given the other reclassification efforts.

For what I can tell, the change in classification will be "focused" on the hereditary branch (ie a lot of classes will be added to the hereditary disease branch). Otherwise, I don't think it would affect any of the refinement work we are doing.
From my perspective (and I haven't reviewed in detail), it is ok to merge.

There are other changes in the file, which I don't know why they are also there:

These changes are spurious (because the file has not been changed in 10 years and the serialisers have changed since then.
I also think this can be merged, but anyone that would like more clarity please reach out!


[Snyk] Fix for 2 vulnerabilities

This PR was automatically created by Snyk using the credentials of a real user.Snyk has created this PR to fix one or more vulnerable packages in the `npm` dependencies of this project.
Changes included in this PR

Changes to the following files to upgrade the vulnerable dependencies to a fixed version:

package.json

Vulnerabilities that will be fixed
With an upgrade:

Severity
Priority Score (*)
Issue
Breaking Change
Exploit Maturity

696/1000  Why? Proof of Concept exploit, Has a fix available, CVSS 7.5
Regular Expression Denial of Service (ReDoS) SNYK-JS-ANSIREGEX-1583908
Yes
Proof of Concept

589/1000  Why? Has a fix available, CVSS 7.5
Prototype Pollution SNYK-JS-UNSETVALUE-2400660
Yes
No Known Exploit

(*) Note that the real score may have changed since the PR was raised.

Commit messages

Package name: check-dependencies
    The new version differs by 55 commits.

03c8847 Tag 2.0.0
65d9ef5 Set Node.js requirement in package.json engines to >=18.3
4917ab0 Simplify the spawn logic
fc04cc8 Drop support for the callback interface
28257dd Tweak ESLint settings
dc16e8a Drop the bluebird devDependency
412337a Drop fs-extra & graceful-fs devDependencies
091279a Drop the findup-sync dependency
10ac9c5 Drop lodash.camelcase & minimist dependencies
35dce52 Update dependencies
2929ba7 Update tested Node.js versions
1f514eb Don't invoke `npm prune`, `npm install` is already enough
65107d2 Drop support for Bower & the `checkCustomPackageNames` option
f28ba1b Avoid `git://` URLs, they're no longer supported
8fdc6ad Limit allowed `packageManager` values
9809f13 Bump word-wrap from 1.2.3 to 1.2.4 (#58)
e522471 Bump semver from 7.3.8 to 7.5.2 (#57)
031416d Bump yaml from 2.2.1 to 2.2.2 (#56)
dff3ab9 Build: Fix running tests on Windows
062005f Build: Update the GitHub Actions config
ecf138f Build: Update dependencies
15c13b3 Build: Remove AppVeyor
89b9df0 Build: Update .gitattributes to files always use LF line endings
db8c172 Build: Tweak GitHub Actions workflow code style

See the full diff

Package name: grunt-contrib-compress
    The new version differs by 5 commits.

bd9fc8e v2.0.0
b8565a9 Update Archiver (#232)
f6815bf Update deps and remove nodeunit loading (#231)
b70c1d9 Update tests and dependencies (#230)
5759edd Bump ini from 1.3.5 to 1.3.7 (#228)

See the full diff

Check the changes in this PR to ensure they won't cause issues with your project.

Note: You are seeing this because you or someone else with access to this repository has authorized Snyk to open fix PRs.
For more information:
🧐 View latest project report
🛠 Adjust project settings
📚 Read more about Snyk's upgrade and patch logic

Learn how to fix vulnerabilities with free interactive lessons:
🦉 Regular Expression Denial of Service (ReDoS)
🦉 Prototype Pollution

Checkmarx One – Scan Summary & Details – 8cd2d4ed-04ef-420b-b765-a8f147126c24
New Issues

Severity
Issue
Source File / Package
Checkmarx Insight

CVE-2022-25883
Npm-semver-7.3.5
Vulnerable Package

CVE-2023-26136
Npm-tough-cookie-2.5.0
Vulnerable Package

CVE-2023-37466
Npm-vm2-3.9.11
Vulnerable Package

CVE-2023-37903
Npm-vm2-3.9.11
Vulnerable Package

CVE-2023-46233
Npm-crypto-js-3.3.0
Vulnerable Package

Passwords And Secrets - Generic Password
/users.yml: 186
Query to find passwords and secrets in infrastructure code.

Passwords And Secrets - Generic Password
/users.yml: 132
Query to find passwords and secrets in infrastructure code.

Passwords And Secrets - Generic Password
/users.yml: 195
Query to find passwords and secrets in infrastructure code.

CVE-2023-44270
Npm-postcss-8.4.5
Vulnerable Package

CVE-2023-44270
Npm-postcss-7.0.39
Vulnerable Package

Unpinned Actions Full Length Commit SHA
/update-challenges-www.yml: 25
Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Unpinned Actions Full Length Commit SHA
/zap_scan.yml: 17
Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Unpinned Actions Full Length Commit SHA
/codeql-analysis.yml: 23
Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Unpinned Actions Full Length Commit SHA
/lint-fixer.yml: 24
Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Unpinned Actions Full Length Commit SHA
/rebase.yml: 17
Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Unpinned Actions Full Length Commit SHA
/ci.yml: 338
Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Unpinned Actions Full Length Commit SHA
/release.yml: 109
Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Unpinned Actions Full Length Commit SHA
/update-news-www.yml: 20
Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Unpinned Actions Full Length Commit SHA
/codeql-analysis.yml: 28
Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Unpinned Actions Full Length Commit SHA
/release.yml: 72
Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Unpinned Actions Full Length Commit SHA
/codeql-analysis.yml: 30
Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Unpinned Actions Full Length Commit SHA
/release.yml: 50
Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Fixed Issues

Severity
Issue
Source File / Package

CVE-2023-26115
Npm-word-wrap-1.2.3

Cx65603961-769c
Npm-debug-2.6.9

CVE-2023-2251
Npm-yaml-1.10.2

Open_Redirect
/routes/keyServer.ts: 11

Open_Redirect
/routes/keyServer.ts: 10

Open_Redirect
/routes/fileServer.ts: 15

Open_Redirect
/routes/fileServer.ts: 14

Open_Redirect
/routes/quarantineServer.ts: 10

Open_Redirect
/routes/quarantineServer.ts: 11

Open_Redirect
/routes/logfileServer.ts: 11

Open_Redirect
/routes/logfileServer.ts: 10


Option to Disable TLS

Closes #3600. Adds the option ``--disable-tls```  to the Charm++ build CLI that disables TLS, which can cause problems in certain contexts (#3600).
If Charm++ or AMPI are built with --disable-tls and try to compile an AMPI program with -tlsglobals, the compiler will return the following:
Error: Charm was built with '--disable-tls' and -tlsglobals is not available.

@nilsdeppe Can you check if this fixes your issue with Python?

Yep, we'll give it a try! Probably @knelli2 will do it and report back. Thanks so much!

Building charm with this branch and the --disable-tls option, I was able to build the SpECTRE python bindings and they work fine. No need to patch charm. So this looks good!


test rerun

Please add a meaningful description for your change here

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

[ ] Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
[ ] Update CHANGES.md with noteworthy changes.
[ ] If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

Run RAT PreCommit

Run Go PreCommit


Attempting to ignore UIRemoteKeyboardWindow when keyboard has never been opened

This is a hacky attempt to fix #152 and #154.
Needs testing.
@dumpzealousys @cozzin
If it works, good.
If it doesn't fix it, then perhaps we need to consider having the "above keyboard" feature controllable (meaning we could let the devs decide if/when the ToastView should be moved to a windowLevel 10000001, or kept at windowLevel 9999999 or similar maximum value normally allowed).

@Coeur I tested it in my situation and it works well. Thank you 👍


⚠ Bofrost.de has degraded performance

In d369397, Bofrost.de (https://www.bofrost.de) experienced degraded performance:

HTTP code: 200
Response time: 810 ms

Resolved: Bofrost.de performance has improved in 556adb2.


move cron jobs

no issue
Changes

This PR moves cron jobs to lambda! And removes the worker from heroku.
Tests

disabled tests for microcredit because they were failing and because some things have changed here https://github.com/impactMarket/backend/pull/629


AIDataMapperCodeAction failed for incomplete check expression

Description:

AIDataMapperCodeAction failed with IllegalStateException

Issue is fixed with https://github.com/ballerina-platform/ballerina-lang/pull/29668

Closing the issue based on the last comment


[WIP] Add an extra parsing pass to properly resolve $id and $ref references.

Up until now the entire schema was parsed in one single pass, but that left some $ref references resolved improperly in some cases. This PR adds an extra pass to resolve these $ref references. And while I'm at it, this PR should also fix all other open issues regarding $ref 🤞.

I don't really know this codebase, but the change seems reasonable to me?
👍

There are a couple of edge cases which are not properly handled by the new code, but which are too complex to get caught by the current unit tests. The new code also allows me to clean up some workarounds in other parts of the code, so this is not yet fully ready to be merged yet.

This seems to fix #200 for me too.

@johandorland are there any pieces I can help out with regarding the unhandled edge cases? Many thanks!

If you can get me a cup of coffee that'd be great, but apart from that I think it's pretty hard to jump in at this moment.
What I'm currently doing is rewriting every "$ref" I encounter with an absolute URI (if applicable). There are however circumstances where a schema is for example describing a property named "$ref", or has an enum which contains an object with a key named "$ref". The parsing code handles this appropriately, but when scanning for "$ref", this logic needs to be duplicated. As an example, this is the current test I'm working on:
{
	"$id" : "http://localhost/schema.json",
	"properties" : {
		"$id" : {
			"$id": "http://localhost/foo.json"
		},
		"$ref" : {
			"const": {
				"$ref" : "hello.world"
			}
		},
		"const" : {
			"$ref" : "#/definitions/$ref"
		}
	},
	"definitions" : {
		"$ref" : {
			"const": {
				"$ref" : "hello.world"
			}
		}
	},
	"dependencies" : {
		"$ref" : "const",
		"const" : "$ref"
	}
}

@johandorland what is going on with the single string value for dependencies?  I don't believe that's part of any version of the spec (and the schema and string array forms get split into two separate keywords in draft-08 for that matter).

@johandorland what is going on with the single string value for dependencies? I don't believe that's part of any version of the spec (and the schema and string array forms get split into two separate keywords in draft-08 for that matter).

You're right, I used an example from https://github.com/json-schema/json-schema/wiki/Dependencies, but I failed to realize it's old and didn't check in the specification. I'll revert this ASAP.

If you can get me a cup of coffee that'd be great, but apart from that I think it's pretty hard to jump in at this moment.

It's on its way! Just let me know the shipping address. 😉
Many thanks for taking care of this.

@johandorland  Question, if you'll pardon my ignorance.. Does this library (with or without this MR) handle recursive references?

@johandorland Question, if you'll pardon my ignorance.. Does this library (with or without this MR) handle recursive references?

Recursive references have always been supported, as long as the recursion is not infinite (something like { "$ref" : "#"} will cause a stack overflow upon validation) However before this MR the $ref mechanics were a bit buggy in general, so in some cases this will fix issues with schemas that happen to be recursive.

Cool, thank you for the info :)


Dangling text file models of deleted files hanging around in memory

VSCode Version: 1.45.1
OS Version: Windows 10

Steps to Reproduce:
Screencast:

Open sample node project below in VS Code.
dnd.zip
Set the setting "typescript.updateImportsOnFileMove.enabled": "always" or "never".
Click to open multiple files (e.g. src/utils/ma.ts, src/utils/mb.ts, src/utils/mc.ts, src/utils/md.ts) in VS Code.
In File Explorer, drag and drop these files back and forth several times, e.g.

1st dnd:  utils/m*.ts -> funcs/m*.ts
2st dnd:  funcs/m*.ts -> utils/m*.ts
3rd dnd:  utils/m*.ts -> funcs/m*.ts

You would see the states for these opened editors are in chaos. Some showed as "deleted", see the screenshot below. But actually these files exist. My initial finding was that vscode didn't clear the opened text models for the old locations after the move operation, so that the next time you move back, vscode will try to reuse the previously cached text models.

Does this issue occur when all extensions are disabled?: Yes

@bpasero sounds like this is on the file UI layer, and not in the UI representation. Can you investigate first? Let me know if you are busy and I can also investigate.

@testforstephen I tried to reproduce but it seemed to work ok for me, maybe more settings are needed? For once, it is not clear to me if it breaks only in the one case where typescript.updateImportsOnFileMove.enabled is set to always or in both cases? It seems quite obvious to me that this issue only surfaces for you if the imports are updated, right?
Can you try the same thing in insiders, to rule out any particular extension or setting you might have that I don't. You can give our preview releases a try from: https://code.visualstudio.com/insiders/

Hm, I still see a bit of weirdness: the title indicates the file is deleted, but the editor tab does not show that. I think there is no breakage of functionality or data loss (@testforstephen correct me if that is different for you) but there seems to be a bug in showing the decoration in that case.

At insider, i can reproduce it in both updateImports enabled and disabled.
I didn't observe data loss yet, but not sure whether some complex case will be.
At least, i observed there are some extra didChange events happening for those problematic editors. And if there is a job on applying the edit in the meantime, it probably throws errors saying there are files changed in the meantime. Especially if you choose to use Refactor Preview panel to preview the updates introduced by dnd, then apply the changes will most likely run into the error saying the file has been modified.

So the fact that imports are still being rewritten when this setting is disabled smells like a bug that I filed as https://github.com/microsoft/vscode/issues/98081
Checking for the state of file models when doing this, I see some stale text file models lurking around, easy to identify:

move those files back and forth as instructed
run the command "Developer: Log Working Copies"
notice how eventually there are working copies for the moved files both in the one folder as well as the other

Checking for callers of ITextModelResolverService#createReferencedObject() it looks like MainThreadDocuments#$tryCreateDocument() is responsible for creating a reference that is not being disposed when the file is moved.
@jrieken is that intentional? I see BoundModelReferenceCollection having some time associated with it, but this means in theory that invalid models are still hanging around when files get deleted or moved? I think that should be revisited and possibly the IWorkingCopyFileService#onDidRunWorkingCopyFileOperation be used to dispose these references.

@jrieken is that intentional? I see BoundModelReferenceCollection having some time associated with it, but this means in theory that invalid models are still hanging around when files get deleted or moved?

Yes - we allow extensions to open text documents and there should be a little respecting of extensions when it comes to dispose. E.g. not dispose immediately but eventually. Also, dispose them when they never appear in an editor or as edited.  So, today they remain for 3 mins or when a certain size constraint is reached.
This code is very ancient and maybe renaming/deleting wasn't an issue back then - if I remember correctly then the editor disposed models outside the ref-counted lifecycle for the longest time, right?

if I remember correctly then the editor disposed models outside the ref-counted lifecycle for the longest time, right?

Maybe in the past, not sure. But for a while file editors use the same mechanisms and dispose the reference when the editor closes:
https://github.com/microsoft/vscode/blob/1fc148b25413ec45e4eb6ce13fa44c0eb813ea4a/src/vs/workbench/contrib/files/common/editors/fileEditorInput.ts#L322
I think just dipsosing the model under the hood without everyone that created a reference knowing about this is very dangerous because you may keep a reference to a model that is disposed without knowing.
I can make a pass over clients of the text model resolver that currently do not react on the move/delete and we can go from there. I will also check if my suggestion about participating in the working copy file event is possible, but I think it should be fine. This would then just be another condition under which we free up a reference, besides the timeout.

Maybe in the past, not sure. But for a while file editors use the same mechanisms and dispose the reference when the editor closes:

https://github.com/microsoft/vscode/blob/e5dadf5e8a00c1b84c77ac4197804967bd393ab5/src/vs/workbench/common/editor/textEditorModel.ts#L96-L98
☝ this is what I have meant. The text editor will re-use a model that it doesn't own so I wonder what happens in the actual owner(s) let it go. E.g this

extension acquires model reference via API
editor opens and reuses that model (line 96)
extension reference is released (e.g after timeout) and model is disposed
editor uses a dispose model?

That cannot happen, text editors go through the reference way as well, otherwise we would have seen a ton of issues by now. Text file editors always create a reference here:
https://github.com/microsoft/vscode/blob/686512400b9cfbe747aa6e94af99797003cb0515/src/vs/workbench/contrib/files/common/editors/fileEditorInput.ts#L253
Still, the fact that we have code like this is dangerous, and possibly not used anymore or at least needs some better understanding. I can look into this as part of looking into the lifecycle of references.

I checked back on the fact that file models and untitled models have their own way of creating text models and it seems fine because ITextModelResolverService is actually going through that code:

untitled: https://github.com/microsoft/vscode/blob/2aa9f3f243b148b6441dc7d51808027dce68d6d2/src/vs/workbench/services/textmodelResolver/common/textModelResolverService.ts#L160
file: https://github.com/microsoft/vscode/blob/2aa9f3f243b148b6441dc7d51808027dce68d6d2/src/vs/workbench/services/textmodelResolver/common/textModelResolverService.ts#L42

There should be no issue from that usage that I can think of as long as everyone is going through these methods which they should.

When i try the latest insider (2020-05-27), i still can reproduce the issue with the sample project above.
Version: 1.46.0-insider (user setup)
Commit: 876f2e70f9a2e1988887f8ca82294418afac15a2
Date: 2020-05-27T05:43:41.115Z
Electron: 7.2.4
Chrome: 78.0.3904.130
Node.js: 12.8.1
V8: 7.8.279.23-electron.0
OS: Windows_NT x64 10.0.18363

Is the fix on the latest insider?

Should be fixed, reopening then.

Using the sample and steps below, this does seems to work for me. Tho, I wonder if we fixed the right issue because the code that @bpasero added isn't actually hit. Well the event fires but there is removal happening because the documents do exist on the extension host but they haven't been requested by an extension and are therefore not kept in the BoundReferenceCollection. This was still a good change but I wonder if we have missed something...
For more insights, I have an extension that logs all onDidOpen and onDidClose events for text documents and this is what it printed. At one point you see a series of CLOSE messages for the files that have been moved, that's followed by OPEN messages for the new locations, and finally come OPEN messages for the files that get updated
OPEN: file:///Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/ma.ts
OPEN: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/ma.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/ma.ts","ref":""}
OPEN: file:///Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/mb.ts
CLOSE: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/ma.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/ma.ts","ref":""}
OPEN: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/mb.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/mb.ts","ref":""}
OPEN: file:///Users/jrieken/Code/_samples/devfest/issues/98057/tsconfig.json
OPEN: file:///Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/mc.ts
CLOSE: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/mb.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/mb.ts","ref":""}
OPEN: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/mc.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/mc.ts","ref":""}
OPEN: file:///Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/md.ts
CLOSE: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/mc.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/mc.ts","ref":""}
OPEN: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/md.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/md.ts","ref":""}
CLOSE: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/md.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/md.ts","ref":""}
OPEN: vscode-userdata:/Users/jrieken/Library/Application Support/code-oss-dev/User/settings.json
CLOSE: vscode-userdata:/Users/jrieken/Library/Application Support/code-oss-dev/User/settings.json
OPEN: vscode-userdata:/Users/jrieken/Library/Application Support/code-oss-dev/User/settings.json
CLOSE: vscode-userdata:/Users/jrieken/Library/Application Support/code-oss-dev/User/settings.json
OPEN: output:tasks
CLOSE: output:tasks
OPEN: output:extension-output-%234
OPEN: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/md.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/md.ts","ref":""}
CLOSE: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/md.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/md.ts","ref":""}
OPEN: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/ma.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/ma.ts","ref":""}
CLOSE: file:///Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/ma.ts
CLOSE: file:///Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/mb.ts
CLOSE: file:///Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/md.ts
CLOSE: file:///Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/mc.ts
CLOSE: file:///Users/jrieken/Code/_samples/devfest/issues/98057/tsconfig.json
OPEN: file:///Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/ma.ts
CLOSE: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/ma.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/utils/ma.ts","ref":""}
OPEN: file:///Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/mb.ts
OPEN: file:///Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/md.ts
OPEN: file:///Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/mc.ts
OPEN: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/ma.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/ma.ts","ref":""}
OPEN: file:///Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/fb.ts
CLOSE: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/ma.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/ma.ts","ref":""}
OPEN: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/fb.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/fb.ts","ref":""}
CLOSE: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/fb.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/fb.ts","ref":""}
OPEN: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/mb.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/mb.ts","ref":""}
CLOSE: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/mb.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/mb.ts","ref":""}
OPEN: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/mc.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/mc.ts","ref":""}
CLOSE: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/mc.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/mc.ts","ref":""}
OPEN: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/md.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/md.ts","ref":""}
CLOSE: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/md.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/md.ts","ref":""}
OPEN: git:/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/ma.ts.git?{"path":"/Users/jrieken/Code/_samples/devfest/issues/98057/src/funcs/ma.ts","ref":""}

@testforstephen Did you try Insiders with this test case or is there a more complicated one in the Java world?

Open the sample nodejs project below in VS Code.
dnd.zip
Set the setting "typescript.updateImportsOnFileMove.enabled": "always".
Click to open multiple files (e.g. src/utils/ma.ts, src/utils/mb.ts, src/utils/mc.ts, src/utils/md.ts) in VS Code.
In File Explorer, drag and drop these files back and forth several times, e.g.

1st dnd:  utils/m*.ts -> funcs/m*.ts
2st dnd:  funcs/m*.ts -> utils/m*.ts
3rd dnd:  utils/m*.ts -> funcs/m*.ts

I tried the steps above in the latest insider 2020-06-03 again, the issue still exists. In fact, i also tried in a new windows machine, it has same issue.
@jrieken Did you check the window title? After DnD back and froth several times, you would see a (deleted) text in the window title.

In Java extension, what i want to do is same as "Update Imports on Move" feature in typescript. A little difference is that we provide the option for users to preview the update. The current problem is that after dragging files back and forth a few times, then applying preview will fail and it says "files were modified in the meantime". At the same time, i noticed there are additional title bar text "(deleted)" appended to these affected editors. Given it can also be reproduced in typescript, that's why i confirm it's a vscode issue.

Oh, my bad. Didn't move back and forth, moved only once

I can now reproduce but from the logs and debugging it seems that the extension host side is looking correct - I see meaningful CLOSE/OPEN messages and I do see how the remove function is executing.
Moving this to June for further investigation but @bpasero I believe there is more to this than just extension referenced documents

A little difference is that we provide the option for users to preview the update.

FYI - our current plan is to add API, e.g a workspace edit can say that it can be previewed and based on user-preference the preview pane will show. Similar to how rename (symbol) preview works

This seems to be the same underlying reason as https://github.com/microsoft/vscode/issues/98653. During the steps we close and open editors, but some of them are inactive. When these get closed (e.g. by doing another DND operation), we do not dispose the model because an editor only disposes a model if it was opened actively, not inactive.
I think there is no issue with disposal of references, but the sequence is rather this:

bulk edit creates model ref and performs edits
bulk edits is done and disposes the ref
we do not dispose the model because it is dirty at this point
at this time we have a model without associated reference that can potentially leak
we open dirty models by default in the background
usually editors participate in the lifecycle of models by creating a reference, but not when opened in background (we do not call resolve in that case)
operation is done again, since files are moved, we close editors and reopen them
closing an inactive editor does not dispose any reference because none was created

I am not sure yet about the proper fix but tested locally that a fix for https://github.com/microsoft/vscode/issues/98653 should also fix this issue. Closing as duplicate.


Game closes when trying to render 3d elements on android port

Basically what the title says, the app closes when trying to select a marble in platinum, when trying to load a stage on platinum and gold and ultra crashes after the garage games logo.
Device: Samsung J7 Neo
Android Version: 9

Do the web versions work?

i tried the web version on my android device, it seems to load the stage without crashing but everything appears invisible except for the skybox, tho when i respawn i can see objects like the starting pad for 1 frame and then everything becomes invisible again.

That's probably why the native versions crash, the game utilizes certain capabilities not supported by your hardware.
Could you visit https://webglreport.com/?v=2 and share what it shows?

Sorry for the delay, here's the report.

It does seem to look fine. Could you try installing older versions of the game and see if they work?

i tried some of the older versions of platinum and they all do the same.

That's unfortunate. As for logs, you could try using https://stackoverflow.com/questions/6854127/filter-logcat-to-get-only-the-messages-from-my-application-in-android. The package name is com.randomityguy.mbhaxe.mbp.


Non-editable test doesn't act as we think

As a result of my recent issues transferring a C Extension project to something skeleton-like and @DominicOram's issues around coverage, I ended up looking into how pytest works for non-editable installs; In short form, it doesn't.
To see this yourself, load up the dev container and run the following:
pip uninstall -y python3-pip-skeleton
pip freeze | xargs pip uninstall -y
pip install .[dev]
mv src src.hidden
pytest

You will get the following output:
/venv/lib/python3.11/site-packages/coverage/report.py:87: CoverageWarning: Couldn't parse '/scratch/enu43627/projects/python3-pip-skeleton/src/python3_pip_skeleton/__init__.py': No source for code: '/scratch/enu43627/projects/python3-pip-skeleton/src/python3_pip_skeleton/__init__.py'. (couldnt-parse)
  coverage._warn(msg, slug="couldnt-parse")

This is because pytest uses it's own import machinery which is reading the files from the src directory regardless of the install being "editable", resulting in very fragile behaviour which is not representative of shipped package

After some discussion with @AlexanderWells-diamond the issue seems to be resolvable with edits to the configuration. Specifically by adding a .coveragerc which sets source = ["src", "**/site-packages/"] and setting --cov-config=.converagerc in in the pytest config. I will look into how we can more cleanly implement these config options and submit a PR once I have a satisfactory resolution

Does this not work then? https://github.com/DiamondLightSource/python3-pip-skeleton/blob/main/pyproject.toml#L86

Coverage.py will read from “pyproject.toml” if TOML support is available, either because you are running on Python 3.11 or later, or because you installed with the toml extra (pip install coverage[toml]). Configuration must be within the [tool.coverage] section, for example, [tool.coverage.run]. Environment variable expansion in values is available, but only within quoted strings, even for non-string values.

https://coverage.readthedocs.io/en/6.5.0/config.html
Relevant?
Might it prefer a multiline string to a list of strings?

Wait, @DominicOram seems to have settings in setup.cfg but it was still failing for him in non-editable mode:

After some discussion with @AlexanderWells-diamond the issue seems to be resolvable with edits to the configuration. Specifically by adding a .coveragerc which sets source = ["src", "**/site-packages/"] and setting --cov-config=.converagerc in in the pytest config. I will look into how we can more cleanly implement these config options and submit a PR once I have a satisfactory resolution

Does this actually fix it in this case? Or does it only work with the editable install?

After some discussion with @AlexanderWells-diamond the issue seems to be resolvable with edits to the configuration. Specifically by adding a .coveragerc which sets source = ["src", "**/site-packages/"] and setting --cov-config=.converagerc in in the pytest config. I will look into how we can more cleanly implement these config options and submit a PR once I have a satisfactory resolution

Does this actually fix it in this case? Or does it only work with the editable install?

This fixes it

Looks like the coverage maintainer uses a similar setup (but not using pytest-cov):

It's also worth trying both pytest and python -m pytest as they have subtly different sys.path values...

Okay, so using .coveragerc for tests I get the follow result:

-e .[dev] works with source = ["src", "*/site-packages"] when src/ is present, but fails when src/ is missing - expected
.[dev] works with source = ["src", "*/site-packages"] regardless of src/ but fails when src/python3_pip_skeleton.egg-info is present

When switching over to pyproject.toml with source = ["src", "*/site-packages"], I get the following results:

-e .[dev] works when src/ is present, but fails when src/ is missing - expected
.[dev] works when src/ is present but fails when src/ is missing or when src/python3_pip_skeleton.egg-info is present

Using source = ["*/site-packages", "src"]

-e .[dev] fails regardless of whether src/ is present - though for different reasons
.[dev] works regardless of src/ but fails when src/python3_pip_skeleton.egg-info is present

From discussion with @coretl, there seems to be two issues here:

coverage reads the config differently from .coveragerc and pyproject.toml, it seems as if only the first path in source is loaded from the toml. This will require fixes upstream
The _version.py file generated by setuptools_scm is included in such a way that it causes an ImportPathMismatchError on non-editable installs. This can be fixed with PY_IGNORE_IMPORTMISMATCH=1pytest.pathlib.ImportPathMismatchError: ('python3_pip_skeleton._version', '/venv/lib/python3.11/site-packages/python3_pip_skeleton/_version.py', PosixPath('/scratch/enu43627/projects/python3-pip-skeleton/src/python3_pip_skeleton/_version.py'))


CoreRT Support (Native compile)

When compiling a service project using CoreRT (native compiler) it won't correct install the Windows service.  CoreRT unfortunately returns an empty string in reference to the  assembly location value.
(public sealed override string Location => string.Empty)
https://github.com/dotnet/corert/issues/5017
This is a related blog indicating why CoreRT returns string.Empty.
https://github.com/dotnet/corert/blob/2a81711d3c71e5dc3dc7a56d33f81e33f5c3f65d/src/System.Private.Reflection.Core/src/System/Reflection/Runtime/General/ThunkedApis.cs#L61
Pertaining to TopShelf when creating the 'CreateTransactedInstaller' it attempts to obtain the service exe path using the assembly.Location.
HostServiceInstaller.cs
static TransactedInstaller CreateTransactedInstaller(Installer installer,string serviceName)
suggestion: We need to determine the service path to the exe/dll however I'm not sure what the best approach would be once it has been compiled using CoreRT.  You might decide a better way to determine the executing assembly location (exe/dll).
if (string.IsNullOrEmpty(assembly.Location))
path = $"/assemblypath={System.IO.Path.Combine(AppDomain.CurrentDomain.BaseDirectory, string.Format("{0}.exe" , serviceName))}";
Environment Setup:
nuget.config (custom nuget feed)

Add Nuget Package reference

Run a native compile using a command prompt.
dotnet publish -r win-x64 -c release
It should produce a native exe in the following directory.
~\bin\Release\netcoreapp2.2\win-x64\native

I'd recommend using the .NET Core Generic Host in this situation, honestly.
Similar to what was done in this sample: https://github.com/MassTransit/Sample-ConsoleService

I am closing all issues that haven't seen any activity. You're welcome to submit a PR to resolve the issue or provide more information.


(PUP-2606) Add EC private key support

Adds two settings key_type and named_curve. If the former is set to ec
then the agent will generate an EC private key. Note this only occurs if the
agent needs to generate a private key, it will have no effect if the agent has a
previously saved RSA key.
Puppet will use the named curve specified by :named_curve, which defaults to
'prime256v1'. Unfortunately ruby does not yet support X25519[1]
Since the saved private key can be either RSA or EC, use the more
generic OpenSSL::PKey.read method which returns the appropriate class
of private key.
[1] https://github.com/ruby/openssl/issues/117

[ ] tests

CLA signed by all contributors.

OpenSSL::PKey::EC#private? was added in https://github.com/ruby/ruby/commit/7c971e61f044dd961517e62a8f41c019db46359e, and private_key? was made an alias to the former. However, OpenSSL::X509::Certificate#check_private_key? assumes OpenSSL::PKey#private? is defined. Need to special case that for ruby < 2.4

Generating a valid EC private key on JRuby doesn't seem to be working, need to look at that

JRuby can't sign a CSR using an EC key:
        4: from org.jruby.ext.openssl.X509Request.sign(X509Request.java:327)
        3: from org.jruby.ext.openssl.impl.PKCS10Request.sign(PKCS10Request.java:140)
        2: from org.bouncycastle.operator.DefaultSignatureAlgorithmIdentifierFinder.find(Unknown Source)
        1: from org.bouncycastle.operator.DefaultSignatureAlgorithmIdentifierFinder.generate(Unknown Source)
Java::JavaLang::IllegalArgumentException (Unknown signature type requested: SHA256WITHNULL)


Remove mistyhacks@ from OWNERS_ALIASES

New role in day job means my ability to participate will be limited. It's been a privilege!

/assign @zacharysarah

Thank you very much for your contributions.

Indeed, many thanks.

@mistyhacks Thanks for your many contributions. 🙇 If you're okay with it, I'd like you to add yourself to emeritus_approvers in the repository's OWNERS file.
/approve

/lgtm


add FontLoader parsing, and add font JSON to local assets

still WIP
FYI, the fonts folder are directly copied from the three.js git repo, first tried remote loading the font js file but git denies access, hence add it local. Not entirely sure about the license but seems OK?

Cool! I think before we go further down this road, we should make sure that TextGeometry is actually the right thing. Do you want your text to be an extruded 3D object (like the demo here: https://threejs.org/docs/#api/en/geometries/TextGeometry ) or just 2D text on a flat surface?
If 2D text is enough, then I bet we could come up with a simpler approach by just drawing text onto a Canvas and then setting that as a texture (option 2 from https://threejs.org/docs/#manual/en/introduction/Creating-text ). That would avoid needing to ship our own fonts, since we could just do something like:
let canvas = document.createElement('canvas');
canvas.width = 400;
canvas.height = 200;
let ctx = canvas.getContext('2d');
ctx.font = '48px sans-serif';
ctx.fillText('Hello world', 10, 10);

and then set that canvas as the texture of a flat surface.
The advantages of the 2D approach are that we don't have to worry about shipping fonts (the browser already knows how to render text, after all) and it would be easy to do things like put text onto some object other than just a flat surface. The advantages of the 3D approach are that it looks awesome and makes the text a real 3D part of the scene.
Are you relying on the 3D-ness of the text in your proposed implementation?

:woman_facepalming: you know what, the 2d option somehow completely escaped me…
Indeed, all I need is to have descriptions aligned (time-wise) with the animations so to avoid post manual editing. 2D is suffice, and come to think of it, perhaps even preferable for the task (3D text might be distracting). Will definitely go with this, thanks Robin.
I’ll close all the PR and issues then?

No problem! Let's at least keep the issues open, and I'm happy to keep helping with option 2. You're definitely not the first person to want text rendering, and it would be great to have.

close since we are attempting a new approach in #43


A cookie associated with a cross-site resource was set without the SameSite attribute

Got this warning message in Chrome, wondering if its going to be an issue in the future?

A cookie associated with a cross-site resource at http://iconify.design/ was set without the SameSite attribute. A future release of Chrome will only deliver cookies with cross-site requests if they are set with SameSite=None and Secure. You can review cookies in developer tools under Application>Storage>Cookies and see more details at https://www.chromestatus.com/feature/5088147346030592 and https://www.chromestatus.com/feature/5633521622188032.

Environment:
Angular 8.2.0
Chrome 78.0
Windows 10
iconify 1.0.3
Thanks a lot

Thanks for noticing. It seems to be new Chrome notice that appears on pretty much every website.
I'm not a fan of tracking, so Iconify doesn't use any cookies and doesn't track anyone.
That cookie notice is not from Iconify, but from CloudFlare. There is nothing I can do about that. There are some threads on CloudFlare forum about it, like this one: https://community.cloudflare.com/t/new-chrome-warning-about-digitrust-cookie-cloudflare-is-setting/118690 so maybe CloudFlare staff will do something about it.
If you really want that cookie or notice to be gone, there are few options:

Try including script from cdn.jsdelivr.net or host it on your server or bundle it with webpack. However this approach will still load icons from API that is also routed through CloudFlare so you'll get cookies from that unless you bundle all icons.
Host your own API instance. Then you'll have full control over everything. See https://github.com/iconify/api.js and https://github.com/iconify/api.php
Use Iconify for React or Iconify for Angular. Those components do not use Iconify script and do not use Iconify API, instead icons are bundled during build process. So your project will not be accessing any external websites, thus no extra cookies.


Request collaborators access

HI @peymanmortazavi
Do you mind giving me collaborators access? I am happy to maintain it.
Thanks
Eric

Hey Eric (@yunpengeric),
Absolutely, thanks for opting for this. I've been suuuper slow in responding to people with other commitments so your help would be more than welcome. I just sent you an invite.
Best,
Peyman


Deploy Performance Tuning Guide

This pull request adds a new Rails Guide, tentatively called "Tuning Performance for Deployment".
A few weeks ago there was an issue to reconsider the default thread count for Puma in Rails and a PR to make the change.
There was some back-and-forth suggesting it would be good to mention some of the information in a performance tuning guide, but no such guide exists. I wrote a quick one and incorporated a lot of feedback from @byroot (Jean Boussier).
If you read this and have opinions about deployment configuration for Puma in a Rails app, I'd love your review!
It's impossible, and undesirable, to have a full-detail guide to all possible Rails deployment options. This guide tries hard to focus on Puma (the default server) and CRuby, and on giving a good start to people who don't already know deployment well. Starting points, not ending points.

Closed in favour of https://github.com/rails/rails/pull/50949


Create bazel-contrib/unused-deps

(see Slack thread)
We'd like to open-source unused-deps, a command-line tool for finding Bazel dependencies which are not actually used in code.
(details) Nick Korostelev from Snapchat suggested we submit it to bazel-contrib, and it seems like a good fit!

Must use an open-source license, preferably Apache-2.0.

We would prefer to use MIT if possible.

Must have wide applicability in the community.

After our BazelCon 2022 talk, about 10 people at various companies have reached out (Snapchat, Roku, etc.) asking us to open-source the tool.
It is useful to most Bazel users who use Java.

Must have a clear point of contact who answers questions from the SIG.

Happy to be that person!

Must be “production quality”:
clear README or other documentation outlining the goal of these rules, how to use them etc.

While we don't have a public repo yet, I copied our README into a gist:
https://gist.github.com/keithl-stripe/870c199d9a1267fda0101cf95ec1f3aa

generated API documentation

This is not an API – do we need this?

include examples of use

Our README shows examples.

tests that are running continuously

Any suggestions for this? Stripe doesn't have any contracts with CI providers.

Must reply to issues/PRs in 2-3 weeks (exact service level agreement TBD)

Happy to comply with this. Internally Stripe has 5-day SLO for this :)

Must have more than one person who is committed to review/approve PRs

@keithl-stripe and @murray-stripe, at minimum, are available from our team.

We recommend encoding this as a CODEOWNERS file.

OK!

Must publish semver releases.

We can do this. Initial release will probably just be 0.1.

Must work with LTS Bazel version

It works with Bazel 5.3.2!

Immediate thought - unused-deps is a generic name that's doesn't scope the product to just java which I believe is the current scope of this tool. Maybe unused-java-deps is a better name, although appreciate that the name follows from the google tools heritage.
I guess implicit in this is the question of how specific to java is this tooling? Could it have a gazelle like plugin ecosystem to allow it to be expanded to other languages?

The tool does three things (roughly):

collect the bazel dependency graph through some bazel queries
collect the java source graph
compare the two graphs

The first and last step are fairly generic, but the middle step is implemented as a compiler plugin to javac. I'm not sure there would be a ton of value in making this pluggable as the other two parts are rather straight-forward.

I'm open to a better name, especially because this one is almost identical to Google's unused_deps tool (which is not supported anymore, and is buggy).
However I imagine someone might want to come along and add Scala and/or Kotlin support, in which case unused-java-deps might be too specific.

In the SIG meeting we decided to create bazel-contrib/unused_jvm_deps

I created unused_jvm_deps as private. I have invited @keithl-stripe and @murray-stripe to the repo. They should be good to proceed with setting up the repository and making it public.

I did not realize that unused-jvm-deps had already been created. This is the repo that will be used.


guilds.forEach is not a function error

I get this error when I'm on the dashboard.
Error:
    9|

    10|
Servers you can manage:

 >> 11| 		  <% guilds.forEach((guild) => { %>
    12| 			<% if (!djsclient.guilds.has(guild.id)) { %>
    13| 			  <a id="invite-btn" target="_blank" href="https://discordapp.com/oauth2/authorize?client_id=492871769485475840&scope=bot&permissions=1506142455&guild_id=<%= guild.id %>">Invite to server
    14|

guilds.forEach is not a function
    at eval (/home/runner/Tsuyo-1/views/servers.ejs:15:15)
    at servers (/home/runner/Tsuyo-1/node_modules/ejs/lib/ejs.js:682:17)
    at tryHandleCache (/home/runner/Tsuyo-1/node_modules/ejs/lib/ejs.js:254:36)
    at View.exports.renderFile [as engine] (/home/runner/Tsuyo-1/node_modules/ejs/lib/ejs.js:485:10)
    at View.render (/home/runner/Tsuyo-1/node_modules/express/lib/view.js:135:8)
    at tryRender (/home/runner/Tsuyo-1/node_modules/express/lib/application.js:640:10)
    at Function.render (/home/runner/Tsuyo-1/node_modules/express/lib/application.js:592:3)
    at ServerResponse.render (/home/runner/Tsuyo-1/node_modules/express/lib/response.js:1012:7)
    at router.get (/home/runner/Tsuyo-1/routes/servers.js:7:7)
    at Layer.handle [as handle_request] (/home/runner/Tsuyo-1/node_modules/express/lib/router/layer.js:95:5)

Does this error show in console automatically or is there a way to replicate it? Also, if possible, which Discord.js version are you running?

it shows on console when I go to the website servers page.

I use repl.it can that be a problem?

Are you using Discord.JS v11? v12 is unstable atm.
As for repl.it, I'm not familiar with it alas. Try running locally via NPM/NodeJS and see if the error still occurs. It's weird that one aspect of the dashboard doesn't work while the rest does hm...

with dashboard disabled no error, And I can't run it off my computer because I have chromebook.

I'm using 12, I can't get 11. Repl.it gets the latest update of the packages automaticly

This error is because you need to add djsclient.guilds.forEach istead of guilds.forEach

I'll try it

It fixed the http://spark.unobot.ga/servers error now there is another error when I go to manage the server.
TypeError: Cannot read property 'hasPermission' of undefined     at router.get (/home/runner/Tsuyo-1/routes/server.js:18:71)     at Layer.handle [as handle_request] (/home/runner/Tsuyo-1/node_modules/express/lib/router/layer.js:95:5)     at next (/home/runner/Tsuyo-1/node_modules/express/lib/router/route.js:137:13)     at Route.dispatch (/home/runner/Tsuyo-1/node_modules/express/lib/router/route.js:112:3)     at Layer.handle [as handle_request] (/home/runner/Tsuyo-1/node_modules/express/lib/router/layer.js:95:5)     at /home/runner/Tsuyo-1/node_modules/express/lib/router/index.js:281:22     at Function.process_params (/home/runner/Tsuyo-1/node_modules/express/lib/router/index.js:335:12)     at next (/home/runner/Tsuyo-1/node_modules/express/lib/router/index.js:275:10)     at Function.handle (/home/runner/Tsuyo-1/node_modules/express/lib/router/index.js:174:3)     at router (/home/runner/Tsuyo-1/node_modules/express/lib/router/index.js:47:12)

with the new bot update I still get the error


[SPARK-11915][SQL] Fix flaky python test pyspark.sql.group

JIRA: https://issues.apache.org/jira/browse/SPARK-11915
We should sort the aggregation results to make the test stable.

Test build #46508 has started for PR 9900 at commit c0788d6.

Test build #46508 has finished for PR 9900 at commit c0788d6.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Looks like it is because I modified InternalRow's hashCode for string in #9883. Close it now.


MD-3142: Disabling test as it attempts to read column with >64K char"

Should we add this as a negative case rather?

Yes we could, once the Drill side changes are in.


Create DOCUMENTATION.md

Created a basic documentation template for the SDK. By no means complete, but hopefully a good starting point.

Thanks! This is great. I'll take care of the nuget account and then edit this to include the actual package name, and then merge it :)

Thanks! We will probably have to adjust some things once we start creating internal links, but hopefully the structure is evident to everyone who will need to push updates to it. I didn't create an issue for this since we had discussed it, but I can add one if you'd like.

My account if needed:
8fd98b788dd2f3efe9c7f990f3b07e308e02092c6ec7c4a42ce8331f5e8e4511


Network Error and Retry builders

This is perfect!
Do we have the error and retry builders? Something like https://pub.dev/packages/flutter_pagewise
Thanks

Something like that?
errorBuilder: (BuildContext context, Object error, CustomListViewState listView) {
  return Column(
    children: <Widget>[
      Text(error.toString()),
      FlatButton(
        onPressed: () => listView.loadMore();
      ),
    ],
  );
}

Added with https://github.com/TheMisir/flutter-listutils/commit/18a63bf9507d2d15384f0b061bb36bbda7afdcfe


Use throttle instead of debounce

First of all, please understand my bad english.
Version
2.3.1
Vue.js version
2.5.13
What is expected?
When we scroll straight down on the mobile browser, @infinite event should be called after passed the given distance.
What is actually happening?
Since scroll event occurs very frequently and it's faster than debounceDuration in the case above, @infinite is being called after 50ms from scroll stoped. Consequently, @infinite is being called at the bottom of content ignoring given distance.
How to reproduce this problem?
In mobile browser, with a container has overflow-scrolling: touch and scroll down to the bottom very fast.
I think it is more common to use throttle function to limit rate of scroll event handler. So I forked this repo and modified debounce to throttle. It fixed the problem above.


RABSW-1097: Pass UserID and GroupID to ClientMount

Signed-off-by: Matt Richerson<EMAIL_ADDRESS>

How did we miss this one? Are we missing a unit test somewhere?

How did we miss this one? Are we missing a unit test somewhere?

It's been on my list of things to do, it just wasn't as important as owner/group for xfs/gfs2 and lustre. There are now 3 different ways that owner/group are set. xfs/gfs2 set it down in nnf-ec when the mkfs is done. Lustre sets it from a temporary lustre client after all the lustre targets are set up. And now raw allocations are done on the compute node by Clientmountd since the chown() will only apply to the current OS.
There's still more work to do too. I have to set the owner/group on the Rabbit node for user containers.


How does FlutterEngineGroup set the click-through effect in the transparent part?

How does FlutterEngineGroup set the click-through effect in the transparent part?
My codes are as follows:
val flutterEngineGroup = FlutterEngineGroup( this )
val point1 = DartExecutor . DartEntrypoint( FlutterInjector.instance() . flutterLoader() . findAppBundlePath() ,  "point1" )
val point1Engine = flutterEngineGroup . createAndRunEngine ( this, point1 )
val flutterView1 = FlutterView ( this, FlutterSurfaceView ( this, true ) )

The second parameter of FlutterSurfaceView is true.
Doing so can only make the background transparent, but cannot click through at the transparent place, thereby triggering the underlying event.
How to achieve it?
doctor:

flutter doctor -v
D:\flutter\bin\flutter.bat doctor --verbose
[√] Flutter (Channel stable, 2.2.1, on Microsoft Windows [Version 10.0.22000.132], locale zh-CN)
    • Flutter version 2.2.1 at D:\flutter
    • Framework revision 02c026b03c (3 months ago), 2021-05-27 12:24:44 -0700
    • Engine revision 0fdb562ac8
    • Dart version 2.13.1
    • Pub download mirror https://pub.flutter-io.cn
    • Flutter download mirror https://storage.flutter-io.cn

[√] Android toolchain - develop for Android devices (Android SDK version 30.0.2)
    • Android SDK at C:\Users\10338\AppData\Local\Android\sdk
    • Platform android-30, build-tools 30.0.2
    • Java binary at: C:\Program Files\Android\Android Studio\jre\bin\java
    • Java version OpenJDK Runtime Environment (build 11.0.8+10-b944.6842174)
    • All Android licenses accepted.

[√] Chrome - develop for the web
    • Chrome at C:\Program Files (x86)\Google\Chrome\Application\chrome.exe

[√] Android Studio
    • Android Studio at C:\Program Files\Android\Android Studio
    • Flutter plugin can be installed from:
       https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
       https://plugins.jetbrains.com/plugin/6351-dart
    • android-studio-dir = C:\Program Files\Android\Android Studio
    • Java version OpenJDK Runtime Environment (build 11.0.8+10-b944.6842174)

[√] IntelliJ IDEA Ultimate Edition (version 2021.1)
    • IntelliJ at D:\idea\IntelliJ IDEA 2021.1.2
    • Flutter plugin version 58.0.3
    • Dart plugin version 211.7727

[√] VS Code (version 1.59.0)
    • VS Code at C:\Users\10338\AppData\Local\Programs\Microsoft VS Code
    • Flutter extension version 3.21.0

[√] Connected device (3 available)
    • Android SDK built for x86 (mobile) • emulator-5554 • android-x86    • Android 10 (API 29) (emulator)
    • Chrome (web)                       • chrome        • web-javascript • Google Chrome 92.0.4515.159
    • Edge (web)                         • edge          • web-javascript • Microsoft Edge 92.0.902.73

• No issues found!
Process finished with exit code 0

Hi @ddxl123
Can you please provide a minimal complete reproducible code sample in a repository
Thank you

Without additional information, we are unfortunately not sure how to resolve this issue.
We are therefore reluctantly going to close this bug for now.
Please don't hesitate to comment on the bug if you have any more information for us; we will reopen it right away!
Thanks for your contribution.
Could everyone who still has this problem please file a new issue with the exact description of what happens, logs, and the output of flutter doctor -v.
All system setups can be slightly different, so it's always better to open new issues and reference related issues.


Migrating data.all assets between Teams - is there a documented process / guide ?

I'd like to create a new team in data.all and move a selection of assets from an existing team to the new team.
For example, we have a team called "Sales" and I want to create a new team called "Sales Retail" as it is more reflective of the business organisation . I want to then move some assets from the "Sales" team to the "Sales Retail" team via a simple process / method.
I am not sure if there is a documented process to do this or a tool we could use , My thoughts are that we could update the metadata database, but there'd still be a lot of remediation needed for other AWS services .
For example what do we need to do with:
glue jobs / existing CKD pipelines and the IAM role assigned to them
lakeformation admins / access
athena workgroups
s3 buckets / datasets
glue databases / tables
I realise there may be more to consider, but I'd like is to have a robust process for this as we'd like to reorganise our existing teams.

Hi @gavclark, thanks for opening an issue! If I understood it correctly this is an issue of usability, you started using data.all with a team with a generic name and as your usage grows you want to use more representative team names. As you forecasted, it has a couple of complications as the team name defines the team IAM role and athena workgroup. As soon as the environment CloudFormation stack is updated (which happens daily), the previous IAM role and workgroup will be deleted and a new ones will be created.
Initial complexity assessment
For the team IAM role:

the new IAM role won't have LakeFormation permissions to team-owned datasets ---> In the current dataset stack (v2.6) the permissions are also granted on stack update, so updating the dataset stacks should update the permissions. We need to verify that the dataset stack gets updated every time we click on "update dataset" in the UI.
the new IAM role won't have LakeFormation permissions to shared datasets --->  If the team has not requested any dataset we won't face any issue. If there are share requests with this team as requested: the share health status is weekly checked and can also be checked on demand, it will appear as UNHEALTHY and the team can reapply the share. Share verify and reapply features are available since 2.3.
the new IAM role won't have IAM permissions to some data.all team created AWS resources. We have to evaluate case by case.

dataset IMPORTED S3 buckets and KMS keys: no problem, the new IAM role will be created alongside new IAM policies granting access to the buckets and KMS keys.
dataset CREATED S3 buckets: in theory it should work without issue; not so sure about KMS keys
other resources. In your case I think you are only interested in the old gluejobs/CDK pipelines. If I remember correctly, the pipelines and jobs use the team role as execution role. I am not sure what will happen when the role gets deleted. It could happen that the update environment fails when trying to delete the role because it is being used in a Glue job (for example). If the environment stack deletes the IAM role then the pipeline resources will be orphan and will fail as the defined execution role does not exist. I don't know if it is possible to update the pipeline stack or if it would require a manual update of all resources to use the new IAM role.

As for the Athena workgroup:

no data.all AWS resources use the workgroup. So we do not need to worry about consumption tools, unless you have developed additional applications.
the team IAM role needs to have IAM permissions to use the workgroup and to access the workgroup S3 location. This should happen automatically during the creation of the new IAM role (update environment stack).

Conclusion: it is a delicate operation with possible unseen issues. We strongly recommend some POC testing in a dev/test environment before carrying this out in prod.
Other options
Automated script using SDK/CLI
If I recognize your user correctly I believe your company is using an older version of data.all; which means this solution might not be available for you. Nevertheless, it is worth mentioning it.
We have implemented an SDK for data.all that allows users to call data.all APIs programmatically using Python. You could use the SDK to automate some of the operations needed:
for all shares:revokeShare, deleteShare, thenupdateEnvironment, updateDataset, for all old shares createShareRequest, addItems, submitShare...
Enhancement to data.all code
Instead of replacing the IAM role and the workgroup, we could use a team label to add the info for the sales retail group. The IAM role will still be 'arn:...."/role:sales' but in data.all it will appear as owned by the Sales Retail group. It might be easier than the actual migration of AWS resources.

Hi @dlpzx , thanks for the reply !
I maybe forgot to mention, we've just recently upgraded to 1.6.2 so we're very far behind current releases :(
To clarify, we'd keep the team for "Sales" , we wouldn't be deleting it or the IAM role associated with it. The idea would be that we'd create a new team "Sales Retail" and migrate some of the assets from "Sales" to "Sales Retail".
I think if we're not deleting the original team, that may simplify things ?. Thanks a lot for your thoughts on the possible complexities, it will help a lot.
Agreed, we definitely need to do a lot of testing in a non-production environment first - I just wanted to check I hadn't missed anything obvious ! .


{{ block.super }} not working?

Please see https://github.com/kylef/Stencil/issues/107
I am opening an issue here as the bug was reproducible for me, but unit tests show it is not an issue with Stencil.

@svanimpe just to check, you are running the 1.8.0 (https://github.com/IBM-Swift/Kitura-StencilTemplateEngine/releases/tag/1.8.0) of  Kitura-StencilTemplateEngine and Stencil 0.8?

Indeed I am.

@svanimpe please see my answer at kylef/Stencil#107 :  https://github.com/kylef/Stencil/issues/107#issuecomment-293365188

Right, that was the issue. Apologies for wasting your time with something this stupid. facepalm

@svanimpe That's OK, no worries :)


Initialise controllers based on new API types

This initialises the controllers for the bootstrap provider and control plane provider based on the two respective CRDs, the OpenShiftBootstrapConfig and the OpenShiftControlPlane types.
This should get us to the point where we can start working on the business logic of the two controllers

/approve
/lgtm


Adds Harvard Westlake School

@hw.com is used for teachers and @hwemail.com is used for students. There is no site hosted at hwemail.com, but there are detailed whois records: https://www.whois.com/whois/hwemail.com

@fishsticks89 Thank you for your patience while waiting for a review and processing your request. Pull request merged.


Sean / meta: update SEO component to accept og properties

Changes:

Update SEO component to accept meta attributes containing og properties if required

Type of change

[ ] Bug fix
[ ] New feature
[x] Update feature
[ ] Refactor code
[ ] Translation to code
[ ] Translation to crowdin
[ ] Script configuration
[ ] Improve performance
[ ] Style only
[ ] Dependency update
[ ] Documentation update
[ ] Release

@sean-binary @mustofa-binary will ALT tags texts be translated?

@sean-binary @mustofa-binary will ALT tags texts be translated?

BeSquare and careers are not a translated pages, hence translation will not be apply to alt tags on these pages


Visibility and clickability of links on white backgrounds an issue for low vision users

Description of the issue
The link style doesn't have a sufficient contrast on a white background for low vision users and there is not enough of a visual cue to the link being clickable.
This was reported by the 'Low vision' tester at the Digital Accessibility Centre (DAC), in an audit commissioned by GOV.UK Notify. They were talking about links in our side-navigation, which have no underline by default, but I think their comments apply equally to all uses of the link style.
They were reporting on this side-navigation:

Their comment was:

When viewing this page, I struggled to see the five links on the left-hand side of the page.
This affects me because of my eye condition the thin light blue text fades into the white
background causing them to become blurry therefore, preventing me from reading the
information provided. It would benefit me that the text is displayed in a darker colour and
made bold so they standout more clearly to read.
The left-hand navigation links are displayed using a light blue contrast with the custom NTA
font. These links used throughout GDS products provide a difficult reading experience for
low vision users as the transparent look to the NTA font impacts text clarity.
With the difficult reading of links, using the mouse to access a required link can prove
problematic. With the mouse pointer frequently being lost, a low vision user heavily relies
on call-to-action focus to ensure correct links are being accessed. Displayed in a list,
navigating this list becomes difficult is no call-to-action focus is noticed.
In this example, an underline is presented on contact with the mouse pointer. The issue I
had with this is that I did not initially see it. The underline is thinly presented and displayed
with the same low contrast transparent blue. The call-to-action focus is not sufficient for
easy notification.
This can be improved by adding weight to the underline to produce a bolder and more
visible focus needed for accurate navigation to which would be welcomed throughout these
pages. Improved call-to-action enhances user/page interaction to increase overall UX of any
website.

I think this can be summarised as the following issues:

the link style on a white background makes the text hard to read
the cursor changing over the clickable area of links is not enough of a cue to know they can be clicked
the link style on a white background makes the underline hard to see

Steps to reproduce the issue
Because the issue seems to occur for users with a visual impairment, I would guess that you would need to test with those types of users.
Actual vs expected behaviour
The tester expected to be able to read the content of the side navigation and identifying them as links but found problems reading the text and knowing they were clickable.
Environment (where applicable)
GOV.UK Notify currently uses version 2.13.0 but the tester mentions experiencing this issue on the www.gov.uk website, so likely they were viewing pages using a more recent version.
The tester was using Google Chrome and Microsoft Edge on Windows (latest versions) for the audit but they mention experiencing this issue on mobile devices and other different screens. See their comment in the description for their exact words.

Could possibly be considered alongside these other issues about the size of click targets:

Tap targets on links within lists are too small
Consider larger elements for a bigger touch/target size